Bug 131497 – [iOS][WK2] Videos should animate into and out of fullscreen.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

[iOS][WK2] Videos should animate into and out of fullscreen.

[Jer Noble]

Created attachment 229058 [details]
Patch

[Jer Noble]

Created attachment 229103 [details]
Patch

[Jer Noble]

Created attachment 229105 [details]
Patch

Now with 100% more changes to WebCore.exp.in.

[Build Bot]

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

[Build Bot]

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

[Eric Carlson]

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+

[Jeremy Jones]

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.

[Simon Fraser (smfr)]

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().

[Jer Noble]

(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.

[Jer Noble]

(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.

[Jer Noble]

Committed r167153: <http://trac.webkit.org/changeset/167153>

[Jer Noble]

Reopening to attach new patch.

[Jer Noble]

Created attachment 229417 [details]
Patch

[Jer Noble]

Whoops, wrong bug.