w3c / screen-orientation

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

BREAKING CHANGE: unlock() returns a pormise #183

Closed marcoscaceres closed 3 years ago

marcoscaceres commented 5 years ago

Closes #104

The following tasks have been completed:

Implementation commitment:


Preview | Diff

marcoscaceres commented 5 years ago

Ok, this is not exactly right... [[defaultOrientation]] is a list, but it should really just be a single value... even if the OS supports multiple.

marcoscaceres commented 5 years ago

@mounirlamouri, could you check in Blink what you actually use for defaultOrientation when unlocking? From a quick search in the Blink code, it seems you also use a single value - but I wasn't able to find where it is defined (does Google have something like searchfox?). In Gecko, we use pass eScreenOrientation_None to the HAL.

We could basically do the same thing in the spec by adding "default" or "none" to the OrientationType enum. The API itself would never return the value, but then we could pass it internally to unlock() to represent the "default orientations".

mounirlamouri commented 5 years ago

The thing is for all orientation locks, the UA can figure out whether we are already in an orientation that matches the lock or whether we should wait for a change. With unlock() or default, the UA would need to know what is the system lock (in most cases, when the manifest has an orientation, we should know). If we can implement this reliably on Android and iOS, I think it would be a fine addition to the spec but I'm not sure if we want to just return immediately.

marcoscaceres commented 5 years ago

If we can implement this reliably on Android and iOS, I think it would be a fine addition to the spec but I'm not sure if we want to just return immediately.

Problem is that we get into situations outlined in the original bug, where screen orientation becomes racy with exitFullScreen() and potentially other things (see @kenchris's issue also). Even if the UA can't determine if the OS changed/unlocked orientation, it should be sufficient to know that everything possible was done by the UA to change the orientation: the task was queued, request was sent to the OS, etc.

mounirlamouri commented 5 years ago

I would be okay to have unlock() return a promise but I think we should have the spec be clearer about the situation. The change as it is assumes that you can apply an orientation lock to the default orientotion which isn't possible actually. #150 kind of touch this issue. Unless we want to solve it first (maybe we do?), one way to resolve this is to simply make the unlock() steps async and have it return a promise when it's done so it would be clear that we are not attempting to notify that the unlock orientation change happened but that the call was registered by the system. A note in the spec making this clear would be good too.