Closed Bug 787534 Opened 7 years ago Closed 7 years ago

Update the Gonk backend to take into account the new screen lock orientations values

Categories

(Core Graveyard :: Widget: Gonk, defect)

All
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-basecamp:-)

RESOLVED FIXED
mozilla18
blocking-basecamp -

People

(Reporter: mounir, Assigned: mounir)

References

Details

Attachments

(1 file)

We should whether return false for currently unsupported screen orientations or make sure we have them supported.
Supporting them shouldn't be hard. We just now have more than 2 setups that can make the phone locked on multiple orientations.

I think we should block on that. Whether with one solution or the other. Otherwise, we might have unexpected behavior if unexpected orientation setups are used.
Is this something an app developer would detect and address during development?  If so it probably doesn't need to block.
We discussed this during triage.  Given the tight B2G schedule, it doesn't seem like something that we'd hold the release for so blocking-basecamp-.  However, that doesn't mean a fix can't land!
blocking-basecamp: ? → -
(In reply to Lucas Adamski from comment #1)
> Is this something an app developer would detect and address during
> development?

There would be no way to detect this programatically which would require web apps developer to whether not use some orientations because they would be known to be failing on B2G or to use UA sniffing to make sure those orientations aren't used on B2G. None of those behaviour should be enforced and advertised by Mozilla. Doing so is hurting the Web.
Given how easy the fix would be, I think it's sad...
Attached patch Tentative patchSplinter Review
This patch is a tentative in the sense that I wasn't able to actually try it because orientation seems pretty broken in Gonk right now.
Assignee: nobody → mounir
Status: NEW → ASSIGNED
Attachment #661823 - Flags: review?(mwu)
Comment on attachment 661823 [details] [diff] [review]
Tentative patch

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

::: widget/gonk/OrientationObserver.cpp
@@ +106,5 @@
>  static nsresult
>  ConvertToScreenRotation(ScreenOrientation aOrientation, uint32_t *aResult)
>  {
>    for (int i = 0; i < ArrayLength(sOrientationMappings); i++) {
> +    if (aOrientation | sOrientationMappings[i].mDomOrientation) {

Is this suppose to be a bitwise and?
(In reply to Michael Wu [:mwu] from comment #5)
> Comment on attachment 661823 [details] [diff] [review]
> Tentative patch
> 
> Review of attachment 661823 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: widget/gonk/OrientationObserver.cpp
> @@ +106,5 @@
> >  static nsresult
> >  ConvertToScreenRotation(ScreenOrientation aOrientation, uint32_t *aResult)
> >  {
> >    for (int i = 0; i < ArrayLength(sOrientationMappings); i++) {
> > +    if (aOrientation | sOrientationMappings[i].mDomOrientation) {
> 
> Is this suppose to be a bitwise and?

Yes. Given that we only use that to know which orientation to use "by default" when lock is called, we can just use the first orientation allowed.
To give some contexts, it's now allowed to have any orientation set and that's expressed with ScreenOrientation being a bitfield.
Comment on attachment 661823 [details] [diff] [review]
Tentative patch

r=me with the operand fixed.
Attachment #661823 - Flags: review?(mwu) → review+
Depends on: 790274
If you are looking for the following commit:
Bug 787534 - Remove Portrait and Landscape special ScreenOrientation values. r=jlebar

It is bug 790274.

Given that our infrastructure suffers a bit, I will not backout this minor patch so the commit message has the correct bug number.
Wrong bug, sorry.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.