Bug 137930

Summary: Fullscreen video dismissal can put application to incorrect orientation on iOS
Product: WebKit Reporter: rabbasian
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, eric.carlson, glenn, jeremyj-wk, jer.noble, jonlee, philipj, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Fix Patch
jer.noble: review+, jer.noble: commit-queue+
Under some circumstances, when a fullscreen video gets dismissed programmatically, it can make the application to have an incorrect interface orientation. none

Description rabbasian 2014-10-21 11:49:42 PDT
Under some circumstances, when a fullscreen video gets dismissed programmatically, it can make the application to have an incorrect interface orientation.
Comment 1 rabbasian 2014-10-21 12:05:59 PDT
Created attachment 240214 [details]
Fix Patch
Comment 2 Darin Adler 2014-10-21 15:54:43 PDT
Comment on attachment 240214 [details]
Fix Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=240214&action=review

> Source/WebCore/ChangeLog:11
> +        When a fullscreen video gets dismissed programmatically, for instance for removing
> +        a webview from the view hierarchy, it can put the device in an incorrect orientation.
> +        Make sure we retain the window of parentView when we attempt to go to fullscreen and use
> +        the retained window for putting the device in correct orientation after dismissal.

Instead of retaining the UIWindow, could we just store the result of calling interfaceOrientation? Or is it helpful to call the interfaceOrientation method at the time of dismissal?
Comment 3 rabbasian 2014-10-21 16:00:58 PDT
(In reply to comment #2)
> Comment on attachment 240214 [details]
> Fix Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=240214&action=review
> 
> > Source/WebCore/ChangeLog:11
> > +        When a fullscreen video gets dismissed programmatically, for instance for removing
> > +        a webview from the view hierarchy, it can put the device in an incorrect orientation.
> > +        Make sure we retain the window of parentView when we attempt to go to fullscreen and use
> > +        the retained window for putting the device in correct orientation after dismissal.
> 
> Instead of retaining the UIWindow, could we just store the result of calling
> interfaceOrientation? Or is it helpful to call the interfaceOrientation
> method at the time of dismissal?

It is necessary that we get the interfaceOrientation at the time of dismissal, because after entering the fullscreen video, the user could start rotating the device and depending on window's view controller rotation handling code, possibly change the orientation of the window.
Comment 4 Jer Noble 2014-10-21 16:16:27 PDT
Comment on attachment 240214 [details]
Fix Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=240214&action=review

>>> Source/WebCore/ChangeLog:11
>>> +        the retained window for putting the device in correct orientation after dismissal.
>> 
>> Instead of retaining the UIWindow, could we just store the result of calling interfaceOrientation? Or is it helpful to call the interfaceOrientation method at the time of dismissal?
> 
> It is necessary that we get the interfaceOrientation at the time of dismissal, because after entering the fullscreen video, the user could start rotating the device and depending on window's view controller rotation handling code, possibly change the orientation of the window.

One alternative would be to use delegate notifications (e.g., - window:willRotateToInterfaceOrientation:duration:) that the window's orientation will change, and save the result.
Comment 5 rabbasian 2014-10-21 16:46:23 PDT
(In reply to comment #4)
> Comment on attachment 240214 [details]
> Fix Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=240214&action=review
> 
> >>> Source/WebCore/ChangeLog:11
> >>> +        the retained window for putting the device in correct orientation after dismissal.
> >> 
> >> Instead of retaining the UIWindow, could we just store the result of calling interfaceOrientation? Or is it helpful to call the interfaceOrientation method at the time of dismissal?
> > 
> > It is necessary that we get the interfaceOrientation at the time of dismissal, because after entering the fullscreen video, the user could start rotating the device and depending on window's view controller rotation handling code, possibly change the orientation of the window.
> 
> One alternative would be to use delegate notifications (e.g., -
> window:willRotateToInterfaceOrientation:duration:) that the window's
> orientation will change, and save the result.

I considered this approach. window:willRotateToInterfaceOrientation:duration: type of callbacks seem to be intended for UIViewController usage and UIViewControllers can implement those methods to get callbacks. Also there is no way to register as the recipient of these calls.

One other alternative approach that I considered is to listen to window rotation change notifications. But this approach would also need the window object since we need to make sure that each time the notification is being received, it is from the window that we are interested in.

I think the approach that I took may be the simplest way to address this issue.
Comment 6 rabbasian 2014-11-04 15:37:55 PST
Created attachment 240963 [details]
Under some circumstances, when a fullscreen video gets dismissed programmatically, it can make the application to have an incorrect interface orientation.

Fixed some commit conflicts.
Comment 7 WebKit Commit Bot 2014-11-04 16:22:41 PST
Comment on attachment 240963 [details]
Under some circumstances, when a fullscreen video gets dismissed programmatically, it can make the application to have an incorrect interface orientation.

Clearing flags on attachment: 240963

Committed r175585: <http://trac.webkit.org/changeset/175585>
Comment 8 WebKit Commit Bot 2014-11-04 16:22:46 PST
All reviewed patches have been landed.  Closing bug.