Summary: | Fullscreen media controls are unusable in pagination mode | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jer Noble <jer.noble> | ||||||||||||
Component: | New Bugs | Assignee: | Jer Noble <jer.noble> | ||||||||||||
Status: | RESOLVED INVALID | ||||||||||||||
Severity: | Normal | CC: | buildbot, commit-queue, eric.carlson, esprehn+autocc, glenn, hyatt, jonlee, kangil.han, kondapallykalyan, philipj, rniwa, sergio, webkit-bug-importer | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Bug Depends on: | 132152 | ||||||||||||||
Bug Blocks: | |||||||||||||||
Attachments: |
|
Description
Jer Noble
2014-04-15 15:06:11 PDT
Created attachment 229448 [details]
Patch
Comment on attachment 229448 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=229448&action=review > Source/WebCore/rendering/RenderFullScreen.h:44 > + RenderBlock* ensurePlaceholder(); > RenderBlock* placeholder() { return m_placeholder; } We recently discussed this idiom on the WebKit mailing list. The coding style we agreed on would be like this: RenderBlock& placeholder(); RenderBlock* placeholderIfExists() { return m_placeholder; } EWS is currently yellow, trying to figure out why these tests are failing: [1/2] fullscreen/full-screen-no-style-sharing.html failed unexpectedly (text diff) [2/2] fullscreen/video-cursor-auto-hide.html failed unexpectedly (text diff) Comment on attachment 229448 [details] Patch Attachment 229448 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5798796424380416 New failing tests: fullscreen/full-screen-no-style-sharing.html fullscreen/video-cursor-auto-hide.html Created attachment 229459 [details]
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
(In reply to comment #4) > (From update of attachment 229448 [details]) > Attachment 229448 [details] did not pass mac-wk2-ews (mac-wk2): > Output: http://webkit-queues.appspot.com/results/5798796424380416 > > New failing tests: > fullscreen/full-screen-no-style-sharing.html This one has some extraneous space in the dump. > fullscreen/video-cursor-auto-hide.html This is the one that's more concerning. Comment on attachment 229448 [details] Patch Attachment 229448 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5532288821493760 New failing tests: fullscreen/full-screen-no-style-sharing.html fullscreen/video-cursor-auto-hide.html Created attachment 229460 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 229448 [details] Patch Attachment 229448 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5976510309597184 New failing tests: fullscreen/full-screen-no-style-sharing.html fullscreen/video-cursor-auto-hide.html Created attachment 229464 [details]
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-02 Port: mac-mountainlion Platform: Mac OS X 10.8.5
(In reply to comment #6) > (In reply to comment #4) > > (From update of attachment 229448 [details] [details]) > > Attachment 229448 [details] [details] did not pass mac-wk2-ews (mac-wk2): > > Output: http://webkit-queues.appspot.com/results/5798796424380416 > > > > New failing tests: > > fullscreen/full-screen-no-style-sharing.html > > This one has some extraneous space in the dump. > > > fullscreen/video-cursor-auto-hide.html > > This is the one that's more concerning. Oh, that test is awful. It's moving the mouse to the middle of the wrapper div, which in full screen mode should put it outside the area of the video. The fact that it worked in the beginning was accidental. I'll update the test and upload a new patch. Created attachment 229491 [details]
Patch for landing.
Comment on attachment 229491 [details] Patch for landing. Rejecting attachment 229491 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 229491, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in LayoutTests/ChangeLog contains OOPS!. Full output: http://webkit-queues.appspot.com/results/4552626645499904 Comment on attachment 229491 [details] Patch for landing. View in context: https://bugs.webkit.org/attachment.cgi?id=229491&action=review > Source/WebCore/rendering/RenderFullScreen.h:43 > + void clearPlaceholder(); > + RenderBlock* ensurePlaceholder(); You forgot to make the name changes Darin suggested. Committed r167441: <http://trac.webkit.org/changeset/167441> This caused a regression where the fullscreen video might not appear at all. Re-opened since this is blocked by bug 132152 Closing as invalid; Hyatt fixed these pagination bugs already. |