Closed Bug 1583837 Opened 3 months ago Closed 3 months ago

The maximum zoom level in Reader Mode is still too small/low

Categories

(Toolkit :: Reader Mode, defect, P1)

69 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla71
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox69 --- wontfix
firefox70 --- verified
firefox71 --- verified

People

(Reporter: Gijs, Assigned: Gijs)

References

(Regression)

Details

(Keywords: access, regression)

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #1578454 +++

(In reply to ciarantreacy1 from bug 1578454 comment #31)

(In reply to :Gijs (he/him) from bug 1578454 comment #29)

I'm struggling to understand the comments here, to be honest. I'm happy to increase the maximum here some more. What I'm struggling with is figuring out what the "right" maximum is so that we don't have to keep changing it. To do that, I'm trying to work out what people were doing in 68 and what the outcome was, so that we can achieve the same outcome here

Thanks Gijs. Sorry for the confusion. In Firefox 68 I had the reader font set to its maximum using the font adjuster on the left side panel. I would then press ctrl + to zoom to the maximum 300%. I'm not sure what the resulting font size was in pt, but it was bigger than the fix in 69.0.1 allows

So as far as I can tell, the pref used to have a maximum of 9, with the maximum font size then being 10 + 9 * 2 = 28px . With 300% zoom that'd be 84px . The other comments in bug 1578454 suggest that really, people want even larger text (like, 127px).

The scaling here is going to become a bit strange. If we increase the maximum of the pref value again from 24 (== 58px) to 37 (84px) or 60 (== 130px) to accommodate that size, users would have to press the [+] button 32 or 55 times to get to that new scaling level.

That's a bit silly. On the other hand, just changing the meaning of the pref will change the font in reader mode for people who have configured it using the current stepping level. Also, people can only "zoom out" (reduce font size) by 4 times from the current default of 5 to 1 - we won't reduce it beyond 12px.

I think we should change the algorithm we use to determine font size based on the pref, such that steps between 9 and 15-20 increase it more drastically (perhaps first by 10px and then by 20px or something along those lines). Changing it by 2px for each step becomes useless at these large font sizes.

FF 68 was fine when it came to max font size in Reader Mode. Why can't the devs just look at the config of FF68 and replicate it? Also, why was it changed in the first place?

Personally, the fix doesn't matter to me anymore because the workaround (noted previously) takes fewer steps than a revised font size. If the max font size is increased, I'd have to go through these 11 steps:
• Click Reader Mode icon
• Press Ctrl +
• Press Ctrl +
• Press Ctrl +
• Press Ctrl +
• Press Ctrl +
• Press Ctrl +
• Press Ctrl +
• Press Ctrl +
• Press Ctrl +
• Press Ctrl +
With the work around, I only have to perform 2 steps:
Click Reader Mode icon
Click 127 px bookmarklet

(In reply to imthepipe from comment #1)

FF 68 was fine when it came to max font size in Reader Mode. Why can't the devs just look at the config of FF68 and replicate it?

Because it used 2 multipliers (font size and page zoom) and now we're only using 1, so either way we have to come up with a way to map the behaviour of 2 multipliers into just 1 multiplier, which means there is always some loss.

Also, why was it changed in the first place?

Because page zoom used to affect the reader mode controls themselves, which people found undesirable - just like page zoom doesn't affect the rest of the browser controls (tabs, location bar, etc.), it should also not affect these controls. So we changed it such that page zoom delegated to reader mode's font size instead, because there was no other way to have full page zoom not zoom the controls - they're in the same document, so all of it gets affected by full page zoom.

Personally, the fix doesn't matter to me anymore because the workaround (noted previously) takes fewer steps than a revised font size. If the max font size is increased

FWIW, I continue to be a bit confused by this because you should only have to do this once - the reader mode font size as affected by the zoom shortcuts should be persisted (in prefs.js, as visible in about:config) across restarts - unless you're also reducing the zoom / font size again all the time?

(the bookmarklet's effects won't persist across restarts as they only affect the document in which you execute the bookmarklet.)

Thanks for the background info - good to know.

Regarding the persistence of Reader Mode zoom - it doesn't happen for me. After switching to Reader Mode, I have to zoom in every time. I do have addons that may limit js. Would this be a possible cause? What is the specific setting in about"config to check?

(In reply to imthepipe from comment #3)

Thanks for the background info - good to know.

Regarding the persistence of Reader Mode zoom - it doesn't happen for me. After switching to Reader Mode, I have to zoom in every time. I do have addons that may limit js. Would this be a possible cause? What is the specific setting in about"config to check?

The same setting as reported in the other bug - reader.font_size. It should track the font size as configured in reader mode (either using shortcuts or using the UI in the reader mode controls).

I checked things again and here's what I found ...
• Opened about:config
• Checked reader.font_size
• It was set to 5
• I change it to 60
• Tested a page in reader mode
• Font was larger, but not large enough
• Went back to about:config
• Checked reader.font_size
• Setting had changed back to 24
• I changed the setting to 127
• Tested another page
• Font size was same as previous test
• Went back to about:config
• It had changed back to 24
Apparently, it can't be set any higher than 24, which appears to be the equivalent of approx 72px.
This may be fine for some users, but personally, a multiplier that yields an equivalent size of 127px would be better for me.
Obviously, the world shouldn't revolve around me, but if user preference has any value, that's mine.
Although not as the current reader mode max font size isn't as large as I need, the setting does seem to be persistent.

:Gijs, since this bug is a regression, could you fill (if possible) the regressed_by field?
For more information, please visit auto_nag documentation.

Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(gijskruitbosch+bugs)
Regressed by: 1557256

(In reply to imthepipe from comment #5)

Apparently, it can't be set any higher than 24, which appears to be the equivalent of approx 72px.

Right, we'll change both of those things in this bug.

Although not as the current reader mode max font size isn't as large as I need, the setting does seem to be persistent.

OK, good, thanks for confirming.

Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/81f97ca78acd
allow more significant increases in font size / zoom in reader mode, more quickly, r=jaws
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71

Comment on attachment 9096574 [details]
Bug 1583837 - allow more significant increases in font size / zoom in reader mode, more quickly, r?jaws

Beta/Release Uplift Approval Request

  • User impact if declined: Regression in zoom sizing in reader mode
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: n/a
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This is a straightforward JS-only, reader-mode-only change
  • String changes made/needed: no
Attachment #9096574 - Flags: approval-mozilla-beta?

Comment on attachment 9096574 [details]
Bug 1583837 - allow more significant increases in font size / zoom in reader mode, more quickly, r?jaws

Allows more zoom, good for a11ty.
OK for uplift for beta 11.

Attachment #9096574 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

Verified that the fonts of the Reader Mode are now increased from 20px to 128px by pressing the keyboard combination Ctrl++ 10 times (the fonts increasing is done as follows: 22px, 24px, 26px, 28px, 32px, 40px, 56px, 72px, 96px, 128px).

Verified as fixed on the latest Nightly 71.0a1 and on the latest Firefox 70 beta 11 on Windows 10, Ubuntu 18.04 and Mac OS X 10.13.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.