The default bug view has changed. See this FAQ.

Make sure unlockOrientation() is a no-op if the orientation wasn't locked

RESOLVED FIXED in Firefox 15

Status

()

Firefox for Android
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mounir, Assigned: mounir)

Tracking

Trunk
Firefox 16
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox13 unaffected, firefox14 wontfix, firefox15 fixed, firefox16 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Created attachment 635242 [details] [diff] [review]
Patch v1
Attachment #635242 - Flags: review?(blassey.bugs)
Comment on attachment 635242 [details] [diff] [review]
Patch v1

Review of attachment 635242 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/GeckoScreenOrientationListener.java
@@ +163,5 @@
>  
>    public void unlockScreenOrientation() {
> +    if (!mIsLocked) {
> +      return;
> +    }

how about?
if (GeckoApp.mAppContext.getRequestedOrientation() == ActivityInfo.SCREEN_ORIENTATION_UNSPECIFIED)
    return;

rather than introduce this new state that needs to be maintained.
(Assignee)

Comment 2

5 years ago
Created attachment 635267 [details] [diff] [review]
Patch v2
Attachment #635242 - Attachment is obsolete: true
Attachment #635242 - Flags: review?(blassey.bugs)
Attachment #635267 - Flags: review?(blassey.bugs)
Attachment #635267 - Flags: review?(blassey.bugs) → review+
(Assignee)

Comment 3

5 years ago
Comment on attachment 635267 [details] [diff] [review]
Patch v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 740190
User impact if declined: with bug 764753, we are going to call unlockOrientation() every time a nsScreen is killed. We want to make sure doing so when there was no orientation lock is a no-op in that case.
Risk to taking this patch (and alternatives if risky): regressions like unlock() not being called but that seems safe enough...
String or UUID changes made by this patch: none
Attachment #635267 - Flags: checkin+
Attachment #635267 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/5a1a4ae9172d
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16

Comment 5

5 years ago
Comment on attachment 635267 [details] [diff] [review]
Patch v2

[Triage Comment]
Low risk, Fennec Native only fix. Approved for Aurora 15.
Attachment #635267 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Updated

5 years ago
status-firefox13: --- → unaffected
status-firefox14: --- → wontfix
status-firefox15: --- → fixed
status-firefox16: --- → fixed
Target Milestone: Firefox 16 → Firefox 15
(Assignee)

Comment 6

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/f770790a44cb

Comment 7

5 years ago
Target Milestone is for m-c.
Target Milestone: Firefox 15 → Firefox 16
You need to log in before you can comment on or make changes to this bug.