REGRESSION (r87622): Scrubbing a Vimeo movie when in fullscreen stops playback; no way to make it start again
Created attachment 95318 [details] Patch
Created attachment 95319 [details] Patch
Created attachment 95324 [details] Patch
Comment on attachment 95324 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95324&action=review > Source/WebCore/ChangeLog:102 > +2011-05-29 Brian Weinstein <bweinstein@apple.com> > + > + Reviewed by Darin Adler. > + > + Controls never hide in full screen after user stops moving mouse > + https://bugs.webkit.org/show_bug.cgi?id=61715 > + <rdar://problem/9522182> > + > + When we get a mouse move event in HTMLMediaElement::defaultEventHandler, and we are in full screen, > + show the media controls, and then start a timer. > + > + The timer fires 3 seconds after the user's last mouse movement (timer is restarted on every mouse > + move), and hides the controls. > + > + * html/HTMLMediaElement.cpp: > + (WebCore::HTMLMediaElement::HTMLMediaElement): Initialize our new timer. > + (WebCore::HTMLMediaElement::play): If we are in full screen mode, start our timer to hide the full screen > + controls. We don't want the user to have to move the mouse to hide them when they use the spacebar > + to play. > + (WebCore::HTMLMediaElement::startHideFullscreenControlsTimer): Starts a oneshot timer 3 seconds in the future > + if we are in full screen. > + (WebCore::HTMLMediaElement::hideFullscreenControlsTimerFired): Make sure that we are currently playing, and > + we are in full screen, and hide the controls. We don't want to hide the controls if we are paused. > + (WebCore::HTMLMediaElement::stopHideFullscreenControlsTimer): Stops the timer. > + (WebCore::HTMLMediaElement::defaultEventHandler): If we get a mouse move event and are in full screen, show the > + controls and start a timer to hide them. > + (WebCore::HTMLMediaElement::enterFullscreen): Start a timer to hide the full screen controls. The user shouldn't > + have the move the mouse once they enter full screen to hide the controls. > + (WebCore::HTMLMediaElement::exitFullscreen): Stop the timer to hide the full screen controls. > + * html/HTMLMediaElement.h: > + * html/shadow/MediaControls.h: Added pure virtual shouldHideControls() method. > + * html/shadow/MediaControlRootElement.cpp: > + (WebCore::MediaControlRootElement::playbackStopped): Stop the timer to hide the full screen controls. > + (WebCore::MediaControlRootElement::shouldHideControls): Added, only report that > + the caller should hide the controls if the panel is not hovered. > + * html/shadow/MediaControlRootElement.h: > + Extra change log entry here. Don't land that!
Comment on attachment 95324 [details] Patch Is this really fixing just bug 61717, or are there changes related to bug 61715 here too?
Created attachment 95326 [details] Patch
Comment on attachment 95326 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95326&action=review > Source/WebCore/html/shadow/MediaControlElements.cpp:653 > > + if (event->type() == eventNames().mouseupEvent) This could be an else
Comment on attachment 95326 [details] Patch We know this is a step in the right direction, but we’re having a lot of problems with the patch and so we can’t land this one. Jer will post a new one once he’s made more progress.
Comment on attachment 95326 [details] Patch My mistake. This is a new version of the patch. Adding back the review+
Comment on attachment 95326 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95326&action=review > Source/WebCore/dom/EventDispatcher.cpp:377 > + if (element->isMediaElement() && shadowRoot && shadowRoot->shadowHost() == element) There is no need to null check shadowRoot here.
Committed r87692: <http://trac.webkit.org/changeset/87692>
Comment on attachment 95326 [details] Patch I'm away from my computer for a few hours. If this patch looks committable as-is, or if another developer wants to land it with changes, that's fine with me.
Comment on attachment 95326 [details] Patch Whoops, missed that commit message. Clearing cq.
This seems like something that should be fixed with the full-screen API spec? Shouldn't it be the case that any element that's full-screen acts in the same way?
(In reply to comment #14) > This seems like something that should be fixed with the full-screen API spec? Shouldn't it be the case that any element that's full-screen acts in the same way? Currently, making a media element full-screen is used to implement the existing full-screen feature for <video>. We need to distinguish this from using the full-screen API on a <video> element; the current behavior is a bug. Once we can distinguish these cases, the full-screen API version won’t want this quirk -- it's specific to the medial-element-specific full-screen. But for now we don't yet distinguish the cases. Not sure that's clear enough. You're right to say there is a bug. But this should not change in the full-screen API spec because this is used to implement something separate from that spec.
Revision r87692 cherry-picked into qtwebkit-2.2 with commit 65741cd <http://gitorious.org/webkit/qtwebkit/commit/65741cd>