[webkit-reviews] review denied: [Bug 30177] Volume slider always starts at half volume : [Attachment 41694] Volume slider patch (rev. 3)

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has denied Hin-Chung Lam
<hclam at google.com>'s request for review:
Bug 30177: Volume slider always starts at half volume
https://bugs.webkit.org/show_bug.cgi?id=30177

Attachment 41694: Volume slider patch (rev. 3)
https://bugs.webkit.org/attachment.cgi?id=41694&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>

> +    String newValue = String::number(m_mediaElement->volume());
> +    if (value() != newValue) {
> +	   setValue(newValue);

String comparison is slower than comparing floats and it seems wasteful to
allocate a String when it won't be used, so I would like to see something like
the following instead:

   float volume = m_mediaElement->volume();
   if (value().toFloat() != volume) {
	setValue(String::number(volume));

Sorry to be so picky :-(