Summary: | [iOS][WK2] Videos should animate into and out of fullscreen. | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jer Noble <jer.noble> | ||||||||||||
Component: | New Bugs | Assignee: | Jer Noble <jer.noble> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | buildbot, commit-queue, eric.carlson, glenn, jonlee, philipj, rniwa, sergio, thorton | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
Jer Noble
2014-04-10 10:08:36 PDT
Created attachment 229058 [details]
Patch
Created attachment 229103 [details]
Patch
Created attachment 229105 [details]
Patch
Now with 100% more changes to WebCore.exp.in.
Comment on attachment 229105 [details] Patch Attachment 229105 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5897126294847488 New failing tests: transitions/cancel-transition.html Created attachment 229112 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 229105 [details]
Patch
r=me but Jeremy should have a close look as well, and *then* you will need to find a WK2 reviewer to give it an official r+
Comment on attachment 229105 [details]
Patch
unofficial r=me. Once nit is that obj-c coordinates are points as doubles. IntRect won't be able to represent every pixel location.
Comment on attachment 229105 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=229105&action=review > Source/WebKit2/WebProcess/ios/WebVideoFullscreenManager.mm:54 > +static IntRect screenRectOfContents(Node* node) > +{ > + if (!node || !node->isElementNode()) Seems like the fullscreen code should be changed to use Element everywhere, right. Is it even possible to take a non-element node fullscreen? Better name for the function would be screenRectForNode(). (In reply to comment #7) > (From update of attachment 229105 [details]) > unofficial r=me. Once nit is that obj-c coordinates are points as doubles. IntRect won't be able to represent every pixel location. Unfortunately, we only have IntRects from our WebCore screen coordinate functions. (In reply to comment #8) > (From update of attachment 229105 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=229105&action=review > > > Source/WebKit2/WebProcess/ios/WebVideoFullscreenManager.mm:54 > > +static IntRect screenRectOfContents(Node* node) > > +{ > > + if (!node || !node->isElementNode()) > > Seems like the fullscreen code should be changed to use Element everywhere, right. Is it even possible to take a non-element node fullscreen? Sure, but that involves a lot of changes to our full screen ChromeClient APIs. We should clean out some of that chaff though. > Better name for the function would be screenRectForNode(). Sure thing. Committed r167153: <http://trac.webkit.org/changeset/167153> Reopening to attach new patch. Created attachment 229417 [details]
Patch
Whoops, wrong bug. |