Bug 214169 – MobileSafari rotates its scene to portrait upside down if it has a PiP on screen

• Component: New Bugs
• Status: RESOLVED FIXED

[Peng Liu]

MobileSafari rotates its scene to portrait upside down if it has a PiP on screen

[Peng Liu]

Created attachment 403937 [details]
Patch

[Peng Liu]

<rdar://problem/64820800>

[Sam Weinig]

Comment on attachment 403937 [details]
Patch

What will it take to make a test for this?

[Jer Noble]

Comment on attachment 403937 [details]
Patch

I'm concerned that this line was in here for a good reason, like allowing a portrait-locked app to open video in landscape fullscreen.

Maybe a better idea is to set `m_viewContrtoller.get()._ignoreAppSupportedOrientations = NO` when we hide the window?

[Peng Liu]

(In reply to Sam Weinig from comment #3)
> Comment on attachment 403937 [details]
> Patch
> 
> What will it take to make a test for this?

We need a full UI test, where we can open a WK2 app, rotate the device into upside-down-mode, and verify that the UI doesn’t flip that way. That cannot be done with a layout test. We might be able to do that with an API test, but we need to fix a bug regarding video fullscreen and picture-in-picture test first (bug 212654).

[Jer Noble]

(In reply to Peng Liu from comment #5)
> (In reply to Sam Weinig from comment #3)
> > Comment on attachment 403937 [details]
> > Patch
> > 
> > What will it take to make a test for this?
> 
> We need a full UI test, where we can open a WK2 app, rotate the device into
> upside-down-mode, and verify that the UI doesn’t flip that way. That cannot
> be done with a layout test. We might be able to do that with an API test,
> but we need to fix a bug regarding video fullscreen and picture-in-picture
> test first (bug 212654).

We need more than API tests, because we can't drive simulated hardware rotations from API tests. XCUITest would be a reasonable framework for doing this, I think, but we don't have any XCUITests set up at the moment. 

XCUITests would let us do a bunch more tests of things like "is the fullscreen airplay button "active" during an airplay session", and the like.

[Peng Liu]

Created attachment 403981 [details]
Patch

[Peng Liu]

(In reply to Jer Noble from comment #4)
> Comment on attachment 403937 [details]
> Patch
> 
> I'm concerned that this line was in here for a good reason, like allowing a
> portrait-locked app to open video in landscape fullscreen.
> 
> Maybe a better idea is to set
> `m_viewContrtoller.get()._ignoreAppSupportedOrientations = NO` when we hide
> the window?

Good point! I think we need to ignore App's supported orientations when a video is in fullscreen. But, when the video is in picture-in-picture, we should NOT ignore that. I have uploaded a new patch.

[Peng Liu]

(In reply to Jer Noble from comment #6)
> (In reply to Peng Liu from comment #5)
> 
> We need more than API tests, because we can't drive simulated hardware
> rotations from API tests. XCUITest would be a reasonable framework for doing
> this, I think, but we don't have any XCUITests set up at the moment. 
> 
> XCUITests would let us do a bunch more tests of things like "is the
> fullscreen airplay button "active" during an airplay session", and the like.

Right!

[EWS]

Committed r264237: <https://trac.webkit.org/changeset/264237>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 403981 [details].