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)
Tracking
()
RESOLVED
FIXED
mozilla18
People
(Reporter: mounir, Assigned: mounir)
References
Details
Attachments
(1 file, 1 obsolete file)
1.22 KB,
patch
|
Details | Diff | Splinter Review |
No description provided.
Attachment #657394 -
Flags: review?(justin.lebar+bug)
Comment 1•13 years ago
|
||
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 2•13 years ago
|
||
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+
Comment 3•13 years ago
|
||
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."
Assignee | ||
Comment 4•13 years ago
|
||
Attachment #657394 -
Attachment is obsolete: true
Assignee | ||
Comment 5•13 years ago
|
||
Target Milestone: --- → mozilla18
Comment 6•13 years ago
|
||
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.
Description
•