[webkit-reviews] review granted: [Bug 88818] Add fullscreen button to Chrome video controls : [Attachment 147569] Use this for review

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Silvia Pfeiffer
<silviapf at chromium.org>'s request for review:
Bug 88818: Add fullscreen button to Chrome video controls
https://bugs.webkit.org/show_bug.cgi?id=88818

Attachment 147569: Use this for review
https://bugs.webkit.org/attachment.cgi?id=147569&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=147569&action=review


> Source/WebCore/html/shadow/MediaControlRootElementChromium.cpp:365
> +	       // When we get a mouse move in fullscreen mode, show the media
controls, and start a timer
> +	       // that will hide the media controls after a 3 seconds without a
mouse move.

Nit: you hide the controls after 2 seconds, not 3.

> Source/WebCore/html/shadow/MediaControlRootElementChromium.cpp:369
> +	   if (m_isFullscreen) {
> +	       // When we get a mouse move in fullscreen mode, show the media
controls, and start a timer
> +	       // that will hide the media controls after a 3 seconds without a
mouse move.
> +	       makeOpaque();
> +	       if (shouldHideControls())
> +		   startHideFullscreenControlsTimer();
> +	   }

Does it matter that you may call startHideFullscreenControlsTimer() multiple
times?