Summary: | Add a mode parameter to MediaControllerInterface::supportsFullscreen() and ChromeClient::supportsVideoFullscreen() | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ada Chan <adachan> | ||||||||||||
Component: | Media | Assignee: | Ada Chan <adachan> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | buildbot, rniwa | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
Ada Chan
2016-01-18 22:16:36 PST
Created attachment 269474 [details]
Patch
Comment on attachment 269474 [details] Patch Attachment 269474 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/722601 New failing tests: media/media-fullscreen-inline.html media/video-controls-visible-exiting-fullscreen.html Created attachment 269481 [details]
Archive of layout-test-results from ews100 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 269474 [details] Patch Attachment 269474 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/722597 New failing tests: media/media-fullscreen-inline.html media/video-controls-visible-exiting-fullscreen.html Created attachment 269483 [details]
Archive of layout-test-results from ews115 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 269474 [details]
Patch
Fixing test failures and will post updated patch soon.
Created attachment 269491 [details]
Patch
Comment on attachment 269491 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=269491&action=review > Source/WebCore/html/HTMLMediaElement.cpp:5300 > + if (!document().page()->chrome().client().supportsVideoFullscreen(m_videoFullscreenMode)) What if document().page() is null? I believe it can be based on the null check a few lines below. (In reply to comment #8) > Comment on attachment 269491 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=269491&action=review > > > Source/WebCore/html/HTMLMediaElement.cpp:5300 > > + if (!document().page()->chrome().client().supportsVideoFullscreen(m_videoFullscreenMode)) > > What if document().page() is null? I believe it can be based on the null > check a few lines below. Good point. I'll move the null check up and post an updated patch. Turns out that early return is not necessary. HTMLVideoElement::setFullscreenMode(mode) should not call HTMLMediaElement::enterfullscreen(mode) if the mode is not supported. I'll revert that code to what it was before. Created attachment 269503 [details]
Patch
Comment on attachment 269503 [details]
Patch
This looks good to me, but a WK2 reviewer will have to give an official r+.
I talked to Sam Weinig who said this doesn't need an official WK2 reviewer's review as this just adds a new parameter. Committed: http://trac.webkit.org/changeset/195487 |