Closed Bug 1356921 Opened 2 years ago Closed 2 years ago

deviceorientation events not firing on devices without gyroscopes

Categories

(Firefox for Android :: General, defect)

55 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 56
Tracking Status
firefox54 --- wontfix
firefox55 --- fixed
firefox56 --- fixed

People

(Reporter: gabybosetti, Assigned: droeh)

References

Details

(Keywords: regression, regressionwindow-wanted)

Attachments

(3 files)

Attached file demo.html
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0
Build ID: 20170321232041

Steps to reproduce:

1) Install fennec 49 onwards (even 55) in Android 6.
2) try to use this application with orientation: http://orb.enclavegames.com/ 
or any other that is suscribed to deviceorientation, as explained here: http://stackoverflow.com/questions/43441602/how-to-get-the-device-orientation-from-a-firefox-for-android-extension

** you can open the demo in the mobile browser and chech it
*It works on other browsers or version 46 


Actual results:

the orientation is never notified


Expected results:

notify the orientation
Status: UNCONFIRMED → RESOLVED
Closed: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1318293
Reopening this and deduping from bug 1318293; it looks like this is an actual regression in how we handle device orientation on phones without gyroscopes.
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: DUPLICATE → ---
Assignee: nobody → droeh
Summary: I can not listen for orientation anymore → deviceorientation events not firing on devices without gyroscopes
Attached file demo2.html
gal007, if you get a chance, can you try out demo2.html (attached on the bug) on your device and let me know what you see? It's the same demo, but listening for "absolutedeviceorientation" rather than "deviceorientation" -- if you get expected results here, then I'm pretty sure we're failing to fall through to other sensors when a gyroscope is not present.
Eugen, flagging you for review on this since you touched it last. I think we need to enable SENSOR_ROTATION_VECTOR events for deviceorientation events for backward compatibility with phones that don't fire SENSOR_GAME_ROTATION_VECTOR events here. Tested by ignoring SENSOR_GAME_ROTATION_VECTOR events in GeckoAppShell.onSensorChanged() -- without the patch we never fire ondeviceorientation, with the patch we fire it as expected.
Attachment #8882608 - Flags: review?(esawin)
Attachment #8882608 - Flags: review?(esawin) → review+
Pushed by droeh@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/758112b2c1fc
Enable SENSOR_ROTATION_VECTOR for DeviceOrientation events. r=esawin
https://hg.mozilla.org/mozilla-central/rev/758112b2c1fc
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Please request Beta approval on this when you get a chance.
Flags: needinfo?(droeh)
Comment on attachment 8882608 [details] [diff] [review]
Enable SENSOR_ROTATION_VECTOR for DeviceOrientation events

Approval Request Comment
[Feature/Bug causing the regression]: 1205649
[User impact if declined]: Device orientation events will not be sent on devices which do not send SENSOR_GAME_ROTATION_VECTOR events
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: No
[Why is the change risky/not risky?]: Should not change behavior in any case except for devices which do not fire SENSOR_GAME_ROTATION_VECTOR events, and fixes behavior for those devices.
[String changes made/needed]:
Flags: needinfo?(droeh)
Attachment #8882608 - Flags: approval-mozilla-beta?
Comment on attachment 8882608 [details] [diff] [review]
Enable SENSOR_ROTATION_VECTOR for DeviceOrientation events

fennec regression fix for some phones, beta55+
Attachment #8882608 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Dylan Roeh (:droeh) from comment #9)
> [Needs manual test from QE? If yes, steps to reproduce]: 
> [List of other uplifts needed for the feature/fix]:

In future please try to answer all the questions in the uplift request template.
See Also: → 1412336
You need to log in before you can comment on or make changes to this bug.