1629030 - [Reader View] Changing the line height while the page is scrolled down hides the dropdown

Status: VERIFIED FIXED

(Toolkit :: Reader Mode, defect, P5)

Description

[Oana Botisan, Desktop QA]

Attachment: reader view.gif

Affected versions

• Firefox 75.0
• Firefox 76.0b3
• Firefox 77.0a1

Affected platforms

• Windows 10 x64
• macOS 10.15
• Ubuntu 18.04 x64

Steps to reproduce

1. Go to https://en.wikipedia.org/wiki/Main_Page.
2. Click on Toggle reader view.
3. Scroll down the page.
4. Click on the Type controls button from the Reader View sidebar.
5. Click on the Decrease/Increase Line Height.

Expected result

• The Type Control menu stays opened.

Actual result

• The Type Control menu closes after one click.

Regression range

• First bad: 2019-01-11
• Last good: 2019-01-10
• Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=74bb778f78793e82cfcae11446387795cb4d4180&tochange=340d5146c4052a47c5aa4f70817dc3ee9fd4e7da

Additional notes

• Please look at the attached gif.
• This issue is reproducing only when you scroll down the page.

Comment 1

[:Gijs (he/him)]

I think the patch in bug 1550836 might fix this.

Note that in both the gif and my testing, the line height still changes - but the dropdown is hidden immediately.

Comment 2

[Mathew Hodson]

The steps from comment #0 still reproduce the issue for me in Nightly build ID 20200513094918 which has the change from bug 1550836.

Comment 3

[:Gijs (he/him)]

Attachment: Bug 1629030 - make line height changes actually take effect in reader mode, r?jaws

The fact that CSS set a fixed line-height on an inner element meant that the
main body of the reader mode text was not changing line height based on the CSS
variable we set on a container higher in the DOM hierarchy.

Comment 4

[:Gijs (he/him)]

Attachment: Bug 1629030 - don't close an open popup when the font changes in reader mode (due to spurious scroll events), r?jaws

This problem only happens if you're scrolled down on the page and then change
font configuration settings (like line height or font size) that cause the
scroll position to change. The scroll event fires and we hide popups in
response. To fix this, we keep a cached height of the body, so that if the
page height has changed since the last scroll event, we don't immediately close
the popup.

Depends on D76310

Comment 5

[:Gijs (he/him)]

Attachment: Bug 1629030 - remove obsolete android code from reader mode, r?jaws

Depends on D76311

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/82160fd55e1f
make line height changes actually take effect in reader mode, r=jaws
https://hg.mozilla.org/integration/autoland/rev/63a158b481ea
don't close an open popup when the font changes in reader mode (due to spurious scroll events), r=jaws
https://hg.mozilla.org/integration/autoland/rev/ae43921b1fe6
remove obsolete android code from reader mode, r=jaws

Comment 7

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/82160fd55e1f
https://hg.mozilla.org/mozilla-central/rev/63a158b481ea
https://hg.mozilla.org/mozilla-central/rev/ae43921b1fe6

Comment 8

[Alexandru Trif, Desktop Test Engineering [:atrif]]

Reproduced the issue using Firefox 77.0a1 (20200410213700) on Windows 10.x64.
The issue is verified fixed with Firefox 78.0b7 (20200612174529) on Windows 10x64, macOS 10.12 and Ubuntu 18.