Closed Bug 1697647 Opened 3 years ago Closed 2 years ago

Support ScreenOrientation.lock

Categories

(GeckoView :: General, enhancement, P1)

Unspecified
All
enhancement

Tracking

(firefox97 fixed)

RESOLVED FIXED
97 Branch
Tracking Status
firefox97 --- fixed

People

(Reporter: agi, Assigned: calu)

References

(Blocks 2 open bugs)

Details

(Keywords: dev-doc-complete, Whiteboard: [geckoview:m97])

Attachments

(1 file)

While working on Bug 1694481, I noticed that we don't support ScreenOrientation.lock, while Fennec did. https://developer.mozilla.org/en-US/docs/Web/API/ScreenOrientation/lock

Whiteboard: [geckoview:m89]
Severity: -- → S3
Priority: -- → P2
Whiteboard: [geckoview:m89] → [geckoview:m90]
Whiteboard: [geckoview:m90] → [geckoview:m91?]
Whiteboard: [geckoview:m91?] → [geckoview:m92?]
Whiteboard: [geckoview:m92?] → [geckoview:m93?]
Assignee: nobody → calu
Attachment #9247602 - Attachment description: Bug 1697647 - Add screen orientation lock api → WIP: Bug 1697647 - Add screen orientation lock api
Depends on: 1714053
Blocks: 1714053
No longer depends on: 1714053
Attachment #9247602 - Attachment description: WIP: Bug 1697647 - Add screen orientation lock api → Bug 1697647 - Add screen orientation lock api
Attachment #9247602 - Attachment description: Bug 1697647 - Add screen orientation lock api → WIP: Bug 1697647 - Add screen orientation lock api
Attachment #9247602 - Attachment description: WIP: Bug 1697647 - Add screen orientation lock api → Bug 1697647 - Add screen orientation lock api
Blocks: 1744125
Priority: P2 → P1
Whiteboard: [geckoview:m93?] → [geckoview:m97]
Pushed by calu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9c623cac6b96
Add screen orientation lock api r=ipc-reviewers,mccr8,agi,smaug,jonalmeida

I updated the changelog which was causing the mozlint failure, and will try to land again.

Flags: needinfo?(calu)
Pushed by calu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/05897abf2c4e
Add screen orientation lock api r=ipc-reviewers,mccr8,agi,smaug,jonalmeida

I updated the web tests to add set a preference dom.screenorientation.allow-lock to true to keep previous behavior. I will try to land again.

Flags: needinfo?(calu)
Pushed by calu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7e588bf3b9a3
Add screen orientation lock api r=ipc-reviewers,mccr8,agi,smaug,jonalmeida
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 97 Branch

This change is in Gecko 97, but CHANGELOG.md change says in 96. :calu, could you move this entry to v97 section?

Depends on: 1746223

(In reply to Makoto Kato [:m_kato] from comment #12)

This change is in Gecko 97, but CHANGELOG.md change says in 96. :calu, could you move this entry to v97 section?

Thanks for pointing this out. I opened a bug and a patch to move it to v97: https://phabricator.services.mozilla.com/D133912

Docs work for this can be tracked in https://github.com/mdn/content/issues/11594

My understanding is that support for ScreenOrientation.lock() disappeared at some point and has now been restored behind a preference. Note that by "supported" I mean "the API exists, even if it can't be used on a particular device".

However if you look at the browser compatibility table on the API there is no indication that it hasn't worked "forever" on all platforms. Can you tell me/give me some indication what version it stopped working on?

(In reply to Hamish Willee from comment #14)

Docs work for this can be tracked in https://github.com/mdn/content/issues/11594

My understanding is that support for ScreenOrientation.lock() disappeared at some point and has now been restored behind a preference. Note that by "supported" I mean "the API exists, even if it can't be used on a particular device".

However if you look at the browser compatibility table on the API there is no indication that it hasn't worked "forever" on all platforms. Can you tell me/give me some indication what version it stopped working on?

From the bugs on bugzilla (https://bugzilla.mozilla.org/show_bug.cgi?id=1681572), looks like it stopped working as early as Dec 9, 2020, version 85.

I would not assume that 85 was the first version to break this. I suspect it was broken on Firefox 69 or so as part of the Fenix rewrite or as part of the Fennec code removal project (bug 1582218). If someone wants to manually bisect there are builds on https://github.com/mozilla-mobile/fenix/releases/

Mozregression is unlikely to work in this case because full Fenix builds are not supported, just the lightweight Geckoview Example browser.

This doesn't work for me on FF97.0a1 Nightly on Google Pixel 3XL, using the test attachment (and after enabling the preference): https://bugzilla.mozilla.org/attachment.cgi?id=9192275
Specifically, I am told that the lock was called, but the promise never resolves.
It does work on Chrome browser.

The basic test here crashed nightly: https://w3c-test.org/screen-orientation/lock-basic.html

(In reply to Hamish Willee from comment #17)

This doesn't work for me on FF97.0a1 Nightly on Google Pixel 3XL, using the test attachment (and after enabling the preference): https://bugzilla.mozilla.org/attachment.cgi?id=9192275
Specifically, I am told that the lock was called, but the promise never resolves.
It does work on Chrome browser.

The basic test here crashed nightly: https://w3c-test.org/screen-orientation/lock-basic.html

Hm, I’m not able to reproduce the message that the lock was called, but promise was not resolved with the additional test demo. The behavior for me is when Lock is clicked, it produces a “SecurityError: The operation is insecure.”. Once fullscreen, the promise is resolved.
If that’s the behavior you’re seeing, this may be resolved by the current patch: https://phabricator.services.mozilla.com/D134872. Otherwise, could you clarify?

Meanwhile, I’ll look into the w3c test, thanks for bringing it up.

@calu

So when I run https://bugzilla.mozilla.org/attachment.cgi?id=9192275 in Chrome and it is in portrait mode, and then put in fullscreen, and then press lock, after a few seconds the screen rotates and locks in the landscape position.

However if I do the same thing in firefox nightly (after enabling the pref) it just sits in portrait forever. The promise DOES resolve if I turn the phone on its side. In other words, it will resolve the promise BUT, it isn't locked - if I turn the phone back to portrait it moves back to portrait.

I have no idea how to build a patch and test it. If you can point to an APK I can try that?

(In reply to Hamish Willee from comment #19)

@calu

So when I run https://bugzilla.mozilla.org/attachment.cgi?id=9192275 in Chrome and it is in portrait mode, and then put in fullscreen, and then press lock, after a few seconds the screen rotates and locks in the landscape position.

However if I do the same thing in firefox nightly (after enabling the pref) it just sits in portrait forever. The promise DOES resolve if I turn the phone on its side. In other words, it will resolve the promise BUT, it isn't locked - if I turn the phone back to portrait it moves back to portrait.

I have no idea how to build a patch and test it. If you can point to an APK I can try that?

Regardless of the patch, I am not seeing that behavior you're mentioning, when I try to reproduce in central/default. Are you able to build firefox mobile on the branch and reproduce the problem?

I can also try to build FF97.0a1 Nightly, but I'm not sure how to do that. If you could point me to instructions or something, I can try to reproduce the problem you're seeing on nightly.

Thanks @calu

I can't build Firefox - I don't know how and I'm not set up for it.

I can also try to build FF97.0a1 Nightly, but I'm not sure how to do that. If you could point me to instructions or something, I can try to reproduce the problem you're seeing on nightly.

I get this from Google Play Store - just enter Firefox Nightly in search and it comes up. The current version obviously changes, but I can still reproduce this error on todays update.

Note, also docs for this have been completed (changes can be tracked fromhttps://github.com/mdn/content/issues/11594)

Hello @calu,
I'm looking to integrate this in Fenix and I want to double check that the fullscreen precondition is checked before OrientationDelegate.onOrientationLock so that we as integrators would get called only when all preconditions are met to just lock the screen orientation and return GeckoResult.allow() everytime.

Flags: needinfo?(calu)

Yes, it will check for fullscreen preconditions in ScreenOrientation::GetLockOrientationPermission (https://searchfox.org/mozilla-central/source/dom/base/ScreenOrientation.cpp#473).

Flags: needinfo?(calu)
You need to log in before you can comment on or make changes to this bug.