Closed Bug 787532 Opened 13 years ago Closed 13 years ago

Update the Android HAL backend to not allow new screen orientation lock values

Categories

(Core :: Hardware Abstraction Layer (HAL), defect)

All
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: mounir, Assigned: mounir)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch Patch (obsolete) — Splinter Review
No description provided.
Attachment #657394 - Flags: review?(justin.lebar+bug)
Is now/new a typo, or am I misunderstanding the summary?
Summary: Update the Android HAL backend to not allow now screen orientation lock values → Update the Android HAL backend to not allow new screen orientation lock values
Comment on attachment 657394 [details] [diff] [review] Patch Roughly speaking, "this/these" refers to something close (in space or time), while "that/those" refers to something far. "This apple I'm holding is green, but that apple you're holding is red." "First we'll do this, then we do that." >@@ -180,18 +180,32 @@ GetCurrentScreenConfiguration(ScreenConf >- bridge->LockScreenOrientation(aOrientation); >- return true; >+ switch (aOrientation) { >+ // The Android backend only supports those orientations. s/those/these >+ case eScreenOrientation_PortraitPrimary: >+ case eScreenOrientation_PortraitSecondary: >+ case eScreenOrientation_Portrait: >+ case eScreenOrientation_LandscapePrimary: >+ case eScreenOrientation_LandscapeSecondary: >+ case eScreenOrientation_Landscape: >+ bridge->LockScreenOrientation(aOrientation); >+ return true; >+ default: >+ return false; >+ } >+ >+ // Some compilers need that even if the switch always return. s/that/this >+ return false; If some compilers need a return outside the switch (really??), then I'd just get rid of the default case. If you take my advice and get rid of the Portrait / Landscape orientations, then this switch statement can have a (PortraitPrimary | PortraitSecondary) case. r=me either way.
Attachment #657394 - Flags: review?(justin.lebar+bug) → review+
The this/that rules are really complex. For example: * "I would like to do here what we do above, but /that/ won't work here." Perhaps it's "that" because you might go on to say "because /this/ case is different." * "Foo.h defines 8 orientations, and the Android backend only supports /those/; it doesn't support /these/ additional orientations." "that/those" are the other guys, the opposing team; "this/these" are the home team, our guys. "I did not have sexual relations with that woman, Miss Lewinsky."
Attached patch PatchSplinter Review
Attachment #657394 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: