Closed
Bug 1472957
Opened 6 years ago
Closed 6 years ago
"Auto" option from Reading Mode is not responsive
Categories
(Firefox for Android Graveyard :: Reader View, defect)
Tracking
(fennec?, firefox61 unaffected, firefox62+ verified, firefox63 verified)
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)
59 bytes,
text/x-review-board-request
|
florian
:
review+
lizzard
:
approval-mozilla-beta+
|
Details |
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
Comment 1•6 years ago
|
||
This relied on the ambient sensor API. See bug 1462308 and bug 1359076 comment 11.
Comment 2•6 years ago
|
||
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)
Assignee | ||
Comment 3•6 years ago
|
||
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)
Assignee | ||
Comment 4•6 years ago
|
||
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.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → jkt
Comment 6•6 years ago
|
||
mozreview-review |
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+
Assignee | ||
Updated•6 years ago
|
Attachment #8990505 -
Flags: review?(bugzilla)
Assignee | ||
Comment 7•6 years ago
|
||
Adding :rnewman just as a precaution as I would like to uplift this if we can also. Thanks Florian
Updated•6 years ago
|
status-firefox61:
--- → unaffected
tracking-firefox62:
--- → +
Comment 8•6 years ago
|
||
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)
Assignee | ||
Updated•6 years ago
|
Attachment #8990505 -
Flags: review?(bugzilla)
Assignee | ||
Comment 9•6 years ago
|
||
Landing as advised, as mentioned it should be fine :). Thanks
Flags: needinfo?(jkt)
Comment 10•6 years ago
|
||
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
Comment 11•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Reporter | ||
Comment 12•6 years ago
|
||
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.
Assignee | ||
Comment 13•6 years ago
|
||
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 14•6 years ago
|
||
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+
Comment 15•6 years ago
|
||
bugherder uplift |
Reporter | ||
Comment 16•6 years ago
|
||
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
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•