search

w3c / screen-orientation

The screen orientation lock specification
https://w3c.github.io/screen-orientation/
Other
27 stars 29 forks source link

Prevent hidden documents from locking orientation #232

Closed marcoscaceres closed 1 year ago

marcoscaceres commented 2 years ago

Closes #221

The following tasks have been completed:

Implementation commitment:

Preview | Diff

annevk commented 2 years ago

I guess this doesn't apply to unlock() because that shouldn't be possible? Maybe assert it? (Although maybe it's possible with manifest apps?)

marcoscaceres commented 2 years ago

I guess this doesn't apply to unlock() because that shouldn't be possible? Maybe assert it? (Although maybe it's possible with manifest apps?)

I was thinking it shouldn't be possible because to hide the page/app it once locked, a user would need to exit fullscreen. But yeah, the assumption doesn't hold when there are multiple virtual desktops.

It may be safer just to prevent it in both cases.

marcoscaceres commented 1 year ago

Prepared test https://github.com/web-platform-tests/wpt/pull/36734

Filed WebKit bug.

@saschanaz or @makotokato, @michaelwasserman, could you please file bugs in your respective engine repos if you support the change?

marcoscaceres commented 1 year ago

Sent a patch for WebKit https://github.com/WebKit/WebKit/pull/6283

marcoscaceres commented 1 year ago

This change has been merged in WebKit.

makotokato commented 1 year ago

Filed https://bugzilla.mozilla.org/show_bug.cgi?id=1802823

makotokato commented 1 year ago

BTW, https://github.com/web-platform-tests/wpt/pull/36734's test (hidden_document.html) is still unclear for me. "hidden documents must not unlock the screen orientation" and "Once maximized, a minimized window can lock or unlock the screen orientation again" don't set pre-lock condition (All browser requires full screen). So I think the latest version of these tests are always failure due to no fullscreen.

Does it forget adding documenttElement.requestFullscreen before orientation.lock call?

makotokato commented 1 year ago

And, why don't we care hidden state in "8.3 Applying an orientation lock" section like fully active if it is in parallel case?

marcoscaceres commented 1 year ago

@makotokato, apologies, just coming back to this.

Thanks for pointing out the issues with the tests and in section "8.3 Applying an orientation lock". I think that was just an oversight. Having a look now.

marcoscaceres commented 1 year ago

@makotokato, about the tests, I've gone over them again and you are right... they were making incorrect assumptions. For example, on macOs at least, it appears one cannot minimize a window once it's in fullscreen. One needs to first exit fullscreen and then the window can be minimized.

Anyway, I'll delete those two tests.

And, why don't we care hidden state in "8.3 Applying an orientation lock" section like fully active if it is in parallel case?

Yeah, good catch. I think instead what needs to happen there is that if the preconditions are not longer met (which is remaining visible and in fullscreen).

marcoscaceres commented 1 year ago

@makotokato, I've filed https://github.com/w3c/screen-orientation/issues/243 to follow up on your comment above. I'll draft up a separate pull request for that.

About the tests, I sent https://github.com/web-platform-tests/wpt/pull/38511