Bug 31135 - Volume slider doesn't have a thumb
Summary: Volume slider doesn't have a thumb
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Hin-Chung Lam
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-04 11:46 PST by Hin-Chung Lam
Modified: 2009-11-04 16:51 PST (History)
3 users (show)

See Also:


Attachments
Patch rev.1 (1.83 KB, patch)
2009-11-04 12:01 PST, Hin-Chung Lam
no flags Details | Formatted Diff | Diff
Patch rev.2 (1.93 KB, patch)
2009-11-04 14:53 PST, Hin-Chung Lam
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hin-Chung Lam 2009-11-04 11:46:56 PST
Using the latest Chromium port, if you mouse over the mute button, the volume slider pops up but it doesn't has a thumb. When you mouse over the slider track there will be a crash.

The reason being that in WebCore/rendering/MediaControlElements.cpp:

void MediaControlVolumeSliderElement::update()
{
    float volume = m_mediaElement->volume();
    if (value().toFloat() != volume) {
        setValue(String::number(volume));
        MediaControlInputElement::update();
    }
}

We don't call MediaControlInputElement::update() which creates the thumb in RenderSlider if the current value of the slider equals the volume. This statement is wrong, because just when a RenderMedia is created, it sets the slider value using its current volume value. So value().toFloat() will always equals to volume when update() is called.

We can force an update just when we call MediaCotnrolVolumeSliderElement::update() during creation of the element, so the if statement would look like  "if (forceUpdate || value().toFloat() != volume)" but the thumb will disappear when we drag it. This is because of MediaControlVolumeSliderElement::defaultEventHandler():

void MediaControlVolumeSliderElement::defaultEventHandler(Event* event)
{
    // Left button is 0. Rejects mouse events not from left button.
    if (event->isMouseEvent() && static_cast<MouseEvent*>(event)->button())
        return;

    MediaControlInputElement::defaultEventHandler(event);

    if (event->type() == eventNames().mouseoverEvent || event->type() == eventNames().mouseoutEvent || event->type() == eventNames().mousemoveEvent)
        return;

    float volume = narrowPrecisionToFloat(value().toDouble());
    if (volume != m_mediaElement->volume()) {
        ExceptionCode ec = 0;
        m_mediaElement->setVolume(volume, ec);
        ASSERT(!ec);
    }
}

We sets the volume of the media element when a mouse event is received, this triggers MediaControlInputElement::update() from RenderMedia::updateControls(). And again, the slider value is the same as the volume so the RenderSlider is never updated.
Comment 1 Hin-Chung Lam 2009-11-04 12:01:58 PST
Created attachment 42506 [details]
Patch rev.1
Comment 2 Hin-Chung Lam 2009-11-04 14:53:18 PST
Created attachment 42523 [details]
Patch rev.2
Comment 3 Hin-Chung Lam 2009-11-04 14:54:04 PST
Update with Eric's suggestions.
Comment 4 Eric Carlson 2009-11-04 16:32:07 PST
Comment on attachment 42523 [details]
Patch rev.2

r=me
Comment 5 WebKit Commit Bot 2009-11-04 16:51:09 PST
Comment on attachment 42523 [details]
Patch rev.2

Clearing flags on attachment: 42523

Committed r50535: <http://trac.webkit.org/changeset/50535>
Comment 6 WebKit Commit Bot 2009-11-04 16:51:14 PST
All reviewed patches have been landed.  Closing bug.