"Aa" from reader mode is not displayed if we select an option

VERIFIED FIXED in Firefox 68

Status

()

defect
P2
normal
VERIFIED FIXED
5 months ago
3 months ago

People

(Reporter: sflorean, Assigned: petru)

Tracking

({regression})

Firefox 66
Firefox 68
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox65 unaffected, firefox66 wontfix, firefox67 wontfix, firefox68 verified)

Details

Attachments

(1 attachment)

Reporter

Description

5 months ago

Environment:
Device: Samsung Galaxy Note 8 (Android 8.0.0);
Build: Nightly 66.0a1 (2019-01-13);

Steps to reproduce:

  1. Go to Wikipedia -> English, or another article with content;
  2. Tap on the Reader Mode icon on URL Bar;
  3. Hide toolbar;
  4. Tap on "Aa" option and choose "+";

Expected result:
"Aa" options are displayed.

Actual result:
"Aa" options are not displayed.

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

Comment 1

5 months ago

Hello, I found a regression range for this bug.
Device: Samsung Galaxy S8 (Android 8.0).
-Last good revision: 4728285e13e47b9c6dfee5af289c32a6f350a9ae
-First bad revision: 045c6ce06b3d2251809d6b51dc780ba7d7d9e156
-Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=4728285e13e47b9c6dfee5af289c32a6f350a9ae&tochange=045c6ce06b3d2251809d6b51dc780ba7d7d9e156

Apparently, it is not 100% reproducible. I was able to reproduce this issue only 3/5 attempts.

Assignee

Updated

4 months ago
Assignee: nobody → petru.lingurar
Status: NEW → ASSIGNED
Assignee

Comment 2

4 months ago

Initially hoped this was a Java dialog but it is not.
Looked over it's implementation but found nothing wrong in the aboutReader family of files.

I see though that there are other open tickets referring to issues with that dialog, like bug 1525805.

Hiro, can you also check this?

Flags: needinfo?(hikezoe)

I can't play the video in comment 0, so it's just a guess, bug 1525805 didn't fix this issue? There is no suspicious commits in the regression range, I suppose it might be bug 1423013 which landed just before the reporter opened this bug. And if bug 1525805 didn't fix this problem, I think it's bug 1527561.

Flags: needinfo?(hikezoe)
Assignee

Comment 4

4 months ago

(In reply to Hiroyuki Ikezoe (:hiro) from comment #3)

I can't play the video in comment 0, so it's just a guess, bug 1525805 didn't fix this issue? There is no suspicious commits in the regression range, I suppose it might be bug 1423013 which landed just before the reporter opened this bug. And if bug 1525805 didn't fix this problem, I think it's bug 1527561.

Same video should be available to anyone with the link - https://drive.google.com/file/d/1MeUgl-mAWLLQs8M0KWcPVDk5Pwv-iDJ4

Just tested on the latest central which contains the patches for the above closed tickets and with just the default settings the issue still reproduces.

Basically, after pressing the font buttons ("+", "-", specific font name) of the reader dialog it will immediately be hidden.
This happens when:

  • _setFontSize executes containerClasses.remove("font-size" + this._fontSize) [1]
  • _setFontType executes bodyClasses.remove(this._fontType) [2]

[1] https://searchfox.org/mozilla-central/rev/b36e97fc776635655e84f2048ff59f38fa8a4626/toolkit/components/reader/AboutReader.jsm#315
[2] https://searchfox.org/mozilla-central/rev/b36e97fc776635655e84f2048ff59f38fa8a4626/toolkit/components/reader/AboutReader.jsm#617

I see this in Firefox 65 though it seems harder to trigger. Rarely the reader view options will close when the user taps on a item in the options. In 67 when it is in this state the options close on just about every tap.

Hiro, do you have further thoughts considering the additional info in comment 4 and 5?

Flags: needinfo?(hikezoe)

Ok, I could reproduce the issue on my phone and debug it a little bit. The unexpected close comes from a mozvisualscroll event. Ccing Jan Henning since he is an expert of visual viewport stuff. I guess changing font-size triggeres a reflow and then the unexpected mozvisualscroll event happens.

Flags: needinfo?(hikezoe)

If you guys need more help, please NI to me. I think what we need to do is to ignore mozvisualscroll events during reflows triggered by the font-size changes.

Re comment# 5, scroll anchoring most likely made this worse. With scroll anchoring, we try to preserve the scroll position relative to the text when zooming, which means that almost every zoom action will effectively also scroll the page. So somehow suppressing the resulting scroll event would indeed seem the way to go here.

Assignee

Comment 10

3 months ago

Changing font size or font type in reader mode would trigger a text reflow.
This coupled with text anchoring means any such action would trigger a scroll
on the page which fires our listener for mozvisualscroll where we close all
dropdowns and hide other visual elements, basically taking users the option to
do consecutive font related changes.

To prevent the font options dissapearing we will ignore any mozvisualscroll
events immediately after (speculative value - 500 ms) any setFontSize() or
setFontType() calls.

Comment 11

3 months ago
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e2921d542d88
Ignore `mozvisualscroll` immediately after setFontSize/setFontType in AboutReader.jsm; r=Gijs

Comment 12

3 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68
Assignee

Updated

3 months ago
Flags: qe-verify+

Comment 13

3 months ago

Hi!

I tested this on the latest version of Nightly 68.0a1 (2019-04-07) with HTC 10 (Android 8.0.0), Samsung Galaxy Note 8 (Android 9), Nexus 9 (Android 7.1.1, Tablet) and I wasn't able to reproduce the issue.

Due to my findings, I'll mark this issue as verified.

Thanks.

Flags: qe-verify+

Updated

3 months ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.