Cancel fullscreen does not automatically adjust the screen orientation

RESOLVED FIXED in Firefox 53

Status

()

defect
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: 709922234, Assigned: jwu)

Tracking

47 Branch
Firefox 53
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

()

Attachments

(2 attachments)

Reporter

Description

3 years ago
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
Reporter

Updated

3 years ago
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!
Reporter

Comment 2

3 years ago
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
Comment hidden (mozreview-request)
: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)
Assignee

Comment 7

3 years ago
(In reply to Sebastian Kaspari (:sebastian) from comment #6)

I have changed the reviewer to "bkelly" on reviewboard.
Assignee

Updated

3 years ago
Flags: needinfo?(topwu.tw) → needinfo?
Assignee

Updated

3 years ago
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)
Assignee

Updated

3 years ago
Attachment #8821951 - Flags: review?(bkelly)
Assignee

Comment 9

3 years ago
(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.
Assignee

Updated

3 years ago
Flags: needinfo?(wchen)

Comment 11

3 years ago
mozreview-review
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)
Assignee

Updated

2 years ago
Keywords: checkin-needed
Autoland can't land this until all pending issues are marked as resolved in MozReview.
Keywords: checkin-needed
Assignee

Updated

2 years ago
Keywords: checkin-needed

Comment 13

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/b706ca522fdd
Always add listener when orientation locked. r=wchen
Keywords: checkin-needed

Comment 14

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b706ca522fdd
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
You need to log in before you can comment on or make changes to this bug.