Closed Bug 766904 Opened 9 years ago Closed 9 years ago

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

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

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

RESOLVED FIXED
Firefox 16
Tracking Status
firefox13 --- unaffected
firefox14 --- wontfix
firefox15 --- fixed
firefox16 --- fixed

People

(Reporter: mounir, Assigned: mounir)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch Patch v1 (obsolete) — Splinter Review
No description provided.
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.
Attached patch Patch v2Splinter Review
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+
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
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16
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+
Target Milestone: Firefox 16 → Firefox 15
Target Milestone is for m-c.
Target Milestone: Firefox 15 → Firefox 16
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.