Closed Bug 1472957 Opened Last year Closed Last year

"Auto" option from Reading Mode is not responsive

Categories

(Firefox for Android :: Reader View, defect)

Firefox 63
ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 63
Tracking Status
fennec ? ---
firefox61 --- unaffected
firefox62 + verified
firefox63 --- verified

People

(Reporter: sflorean, Assigned: jkt)

References

Details

(Keywords: regression)

Attachments

(1 file)

Environment: 
Device: Samsung Galaxy Note8 (Android 8.0.0);
Build: Nightly 63.0a1 (2018-07-02);

Steps to reproduce:
1. Go to Wikipedia -> English;
2. Tap on the Reader Mode icon on URL Bar;
3. Tap on "Aa" option;
4. Select "Dark";
5. Change to "Auto" option.

Expected result:
Light theme is applied.

Actual result:
Dark theme is displayed, "auto" option is not responsive.

Notes:
See the video: https://drive.google.com/open?id=1_QIjt2dBflIwiDC089I4WbtbH_07Jrs7

Regression window:
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b996cabc7ef54bbe050d647494bf00d668ec52e6&tochange=0f8f71b0b9d84e7732c07f841e395de516b31b66
This relied on the ambient sensor API. See bug 1462308 and bug 1359076 comment 11.
jkt, do you know if we ever filed a follow up bug (or came up with a plan) for reader mode? https://bugzilla.mozilla.org/show_bug.cgi?id=1359076#c30

thanks
Flags: needinfo?(jkt)
Ah I'm sorry I dunno how this got missed.

The original idea was to either drop the feature or allow it only for this page/chrome context.

The video does make it a little worse than it actually is. The UI is still responsive after the user presses that button.

The code in https://searchfox.org/mozilla-central/rev/fd5c37f1dd9a0d1e327a6c6b4d81ea92f52c4330/dom/system/nsDeviceSensors.cpp#593 should be able to handle if the API is called from a priv context.

This will impact stable on the next release (62), if we think it's urgent enough perhaps we should uplift removing that button?
Flags: needinfo?(jkt)
The simple uplift would be commenting out the pref in:

mobile/android/app/mobile.js
  from pref("reader.color_scheme.values", "[\"dark\",\"auto\",\"light\"]");

to use
  modules/libpref/init/all.js
  value pref("reader.color_scheme.values", "[\"light\",\"dark\",\"sepia\"]");

This would keep the interface looking the same and be pretty risk free until we decide on a better plan here.
Assignee: nobody → jkt
Comment on attachment 8990505 [details]
Bug 1472957 - Disable auto reader mode for mobile as device light doens't work.

https://reviewboard.mozilla.org/r/255578/#review262472

I'm not a peer for mobile, but this is so simple that I can rs it if this is what you were looking for.
Attachment #8990505 - Flags: review?(florian) → review+
Attachment #8990505 - Flags: review?(bugzilla)
Adding :rnewman just as a precaution as I would like to uplift this if we can also. Thanks Florian
Jonathan maybe it's enough to land it and then ask QA to verify on nightly? Or, is there someone else who can review?
Flags: needinfo?(jkt)
Attachment #8990505 - Flags: review?(bugzilla)
Landing as advised, as mentioned it should be fine :). Thanks
Flags: needinfo?(jkt)
Pushed by jkingston@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/46d48232c540
Disable auto reader mode for mobile as device light doens't work. r=florian
https://hg.mozilla.org/mozilla-central/rev/46d48232c540
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Tested with Nokia 6(Android 7.1.1) so verified as fixed on the latest Nightly build (07-19).
We have now dark, light, sepia options and changing from dark to sepia makes the light theme to be applied.
Comment on attachment 8990505 [details]
Bug 1472957 - Disable auto reader mode for mobile as device light doens't work.

Approval Request Comment
[Feature/Bug causing the regression]: 1359076
[User impact if declined]: Broken button on fenec reader mode
[Is this code covered by automated tests?]: N/A
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: No
[Is the change risky?]: No
[Why is the change risky/not risky?]: It's a pref change
[String changes made/needed]: N/A
Attachment #8990505 - Flags: approval-mozilla-beta?
Comment on attachment 8990505 [details]
Bug 1472957 - Disable auto reader mode for mobile as device light doens't work.

Simple fix to set the theme for reader mode. Let's take this for beta 11.
Attachment #8990505 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Verified as fixed ob build 62 Beta 11.
Devices: Nexus 5(Android 6.0.1) and Samsung Galaxy Note8 (Android 8.0)
Status: RESOLVED → VERIFIED
Depends on: 1495344
You need to log in before you can comment on or make changes to this bug.