Cancel fullscreen does not automatically adjust the screen orientation

RESOLVED FIXED in Firefox 53

Status

()

Firefox for Android
General
RESOLVED FIXED
10 months ago
3 months ago

People

(Reporter: 709922234, Assigned: jwu)

Tracking

47 Branch
Firefox 53
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

(URL)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

10 months ago
Created attachment 8769473 [details]
ScreenRecord_2016-07-10-18-44-20_000.mp4

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

10 months 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

10 months 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

4 months ago
(In reply to Sebastian Kaspari (:sebastian) from comment #6)

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

Updated

4 months ago
Flags: needinfo?(topwu.tw) → needinfo?
(Assignee)

Updated

4 months 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

4 months ago
Attachment #8821951 - Flags: review?(bkelly)
(Assignee)

Comment 9

4 months 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

4 months ago
Flags: needinfo?(wchen)

Comment 11

4 months 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+

Updated

4 months ago
Flags: needinfo?(wchen)
(Assignee)

Updated

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

Updated

3 months ago
Keywords: checkin-needed

Comment 13

3 months 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

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b706ca522fdd
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
You need to log in before you can comment on or make changes to this bug.