Closed Bug 1285802 Opened 4 years ago Closed 4 years ago

Cancel fullscreen does not automatically adjust the screen orientation

Categories

(Firefox for Android :: General, defect)

47 Branch
ARM
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 53
Tracking Status
firefox53 --- fixed

People

(Reporter: 709922234, Assigned: jwu)

References

()

Details

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:47.0) Gecko/20100101 Firefox/47.0
Build ID: 20160623154057

Steps to reproduce:

1. enter full screen
2. screen.orientation.lock('landscape')
3. Click the back button to exit fullscreen
4. repeat 1,2,3


Actual results:

Firefox mobile screen orientation is landscape


Expected results:

screen orientation is portrait
Component: Untriaged → Locale switching and selection
OS: Unspecified → Android
Product: Firefox → Firefox for Android
Hardware: Unspecified → ARM
Hi,
What device did you used when this issue appeared?
What Android version does it have?
Thanks!
Sony Xperia Z5C 
Andriod 6.0.1
Not a language selection bug.
Component: Locale switching and selection → General
Summary: [mobile] Cancel fullscreen does not automatically adjust the screen orientation → Cancel fullscreen does not automatically adjust the screen orientation
:jwu is still working on this (and waiting for review).
Assignee: nobody → topwu.tw
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
I would like to redirect the review to Ben Kelly because I'm not familiar with the DOM code and someone working on this module should review this change.

However reviewboard is complaining:
> Unable to update reviewers as the review request has pending changes (the patch author has a draft)

jwu: Do you still have a draft of an updated version that you need to publish on reviewboard? If you are going to publish the review request then just change the reviewer to "bkelly" before doing that. Thanks!
Flags: needinfo?(topwu.tw)
(In reply to Sebastian Kaspari (:sebastian) from comment #6)

I have changed the reviewer to "bkelly" on reviewboard.
Flags: needinfo?(topwu.tw) → needinfo?
Flags: needinfo?
Comment on attachment 8821951 [details]
Bug 1285802 - Always add listener when orientation locked.

Did you press 'publish'? Seems like I'm still set as reviewer.
Attachment #8821951 - Flags: review?(s.kaspari)
Attachment #8821951 - Flags: review?(bkelly)
(In reply to Sebastian Kaspari (:sebastian) from comment #8)
Sorry, the change has be published.
Attachment #8821951 - Flags: review?(bkelly) → review?(wchen)
Sorry, I don't know this code either.  William, can you take a look?

It seems this change will make us fire system events more often and its unclear to me if that is ok.
Flags: needinfo?(wchen)
Comment on attachment 8821951 [details]
Bug 1285802 - Always add listener when orientation locked.

https://reviewboard.mozilla.org/r/101030/#review104460

::: dom/base/ScreenOrientation.cpp:376
(Diff revision 1)
> -  if (aIsFullScreen && !mFullScreenListener) {
> +  if (aIsFullScreen) {
> +    if (!mFullScreenListener) {
> -    mFullScreenListener = new FullScreenEventListener();
> +      mFullScreenListener = new FullScreenEventListener();
> +    }
> +
>      aRv = target->AddSystemEventListener(NS_LITERAL_STRING("fullscreenchange"),

It looks like AddSystemEventListener ignores duplicate attempts to add the same event listener with the same parameters so this should be fine.
Attachment #8821951 - Flags: review?(wchen) → review+
Flags: needinfo?(wchen)
Keywords: checkin-needed
Autoland can't land this until all pending issues are marked as resolved in MozReview.
Keywords: checkin-needed
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/b706ca522fdd
Always add listener when orientation locked. r=wchen
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b706ca522fdd
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
You need to log in before you can comment on or make changes to this bug.