| Summary: | MobileSafari rotates its scene to portrait upside down if it has a PiP on screen | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Peng Liu <peng.liu6> | ||||||
| Component: | New Bugs | Assignee: | Peng Liu <peng.liu6> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | eric.carlson, ews-watchlist, glenn, jer.noble, philipj, sam, sergio, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Peng Liu
2020-07-09 17:45:42 PDT
Created attachment 403937 [details]
Patch
Comment on attachment 403937 [details]
Patch
What will it take to make a test for this?
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?
(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). (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. Created attachment 403981 [details]
Patch
(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. (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! Committed r264237: <https://trac.webkit.org/changeset/264237> All reviewed patches have been landed. Closing bug and clearing flags on attachment 403981 [details]. |