1563484 - "Zoom Text Only" does not zoom system fonts (stylo regression)

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Fanolian]

Attachment: Zoom text only.png

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:69.0) Gecko/20100101 Firefox/69.0
Build ID: 20190703162038

P.S. layout.css.servo.chrome.enabled was removed in 2018-03-22 build by bug 1447611. To test for regression, please use a Nightly build between 2017-10-28 and 2018-03-21.

Steps to reproduce:

1. Start current Nightly (2019-07-03) in a new profile.
2. Go to about:support.
3. Try to zoom the page either by Ctrl +/-, Ctrl-mousewheel, or by the buttons in hamburger menu.
   ↳ All text should be zoomed by now.
4. Enable Zoom Text Only from Menu bar (F10) > View > Zoom > ☑ Zoom Text Only.
5. Zoom the page again.

Actual result:

Some text is not zoomable, e.g. text in the tables. (Please refer to the attached screenshot.)

Expected result:

All text should be zoomable.

Notes

Only some about: pages are affected. E.g.
about:support
about:memory
about:studies
about:credits (This actually redirects to https://www.mozilla.org/credits/ but the web page is still affected.)

For those affected pages, they can be further divided into 2 categories.

1. Affected (non-zoomable) text can be made zoomable. Example: about:support:
   a. In about:support, inspect an affected text, e.g. Build ID.
   b. Open Fonts Inspector and change some of the font parameters.
   ↳ The changes are correctly reflected in the text.
   c. Zoom the page again.
   ↳ Build ID is now zoomable. Reloading the page resets the behaviour though.

2. Affected text cannot be made zoomable. Example: about:memory:
   a. In about:memory, inspect an affected text, e.g. Show memory reports.
   b. Open Fonts Inspector and change some of the font parameters.
   ↳ The changes are not reflected in the text.
   ↳ Browser Console shows this error:
   Content Security Policy: The page’s settings blocked the loading of a resource at inline (“default-src”).
   c. Zoom the page again.
   ↳ Show memory reports is still not zoomable.

Comment 1

[Fanolian]

From Mozgression,
Last good Nightly: 2018-02-19
First bad Nightly: 2018-02-20
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=4e4ea471e2d1c8db0c5f3a766bcb989bf95b76ad&tochange=861067332bac96a44bbf41ef366f58a30476057b

This is regressed by bug 1417138. I confirm this by manually setting layout.css.servo.chrome.enabled to false.
Since bug 1417138 was only about flipping a preference, the earliest build that the issue can be reproduced was 2017-10-28 which contained bug 1411532. It added layout.css.servo.chrome.enabled (disabled by default) to Nightly. I confirm it by setting layout.css.servo.chrome.enabled to true manually.

As mentioned in comment 0, layout.css.servo.chrome.enabled was removed in 2018-03-22 build by bug 1447611.

I set Firefox 67,68 as wontfix since it is a longstanding bug.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

So the issue here is system fonts:

• data:text/html,<div style="font: message-box">not zoomable</div><div>zoomable</div>

David, you've objected to doing other sort of scaling of system fonts (like bug 1336558).

I guess zoom is kind different than that, but just want to make sure you don't have any objection to apply text-only zoom to system fonts. This should be an easy fix otherwise.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Well guess I'll ask for review rather than permission :)

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1563484 - Apply text-only zoom to system fonts. r=dbaron

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1563484 - Add reftest support for text-zoom. r=dbaron

Seems we had no reftests for this feature :(

Depends on D36893

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1563484 - Add reftests for this bug. r=dbaron

Depends on D36894

Comment 7

[David Baron :dbaron:]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)

I guess zoom is kind different than that, but just want to make sure you don't have any objection to apply text-only zoom to system fonts. This should be an easy fix otherwise.

Yeah, zoom is different; if the user zooms the page, the fonts should zoom. It's different from a default behavior of scaling the system fonts to a different size.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ea1d0ea4b5e5
Apply text-only zoom to system fonts. r=dbaron
https://hg.mozilla.org/integration/autoland/rev/fe26c79a0191
Add reftest support for text-zoom. r=dbaron
https://hg.mozilla.org/integration/autoland/rev/139c0aa658a1
Add reftests for this bug. r=dbaron

Comment 9

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/ea1d0ea4b5e5
https://hg.mozilla.org/mozilla-central/rev/fe26c79a0191
https://hg.mozilla.org/mozilla-central/rev/139c0aa658a1

Comment 10

[Fanolian]

Thank you.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Given that it's a one-liner with tests, is this something we might want to take in ESR68?

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9075922 [details]
Bug 1563484 - Apply text-only zoom to system fonts. r=dbaron

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: stylo regression that may affect accessibility of about: pages for users that use text-only zoom.
• User impact if declined: Text-only zoom doesn't seem to work in pages that use system fonts. Note that default zoom is not text zoom.
• Fix Landed on Version: 69
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): One-liner with lots of tests, since we didn't have any for this feature.
• String or UUID changes made by this patch: none

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9075922 [details]
Bug 1563484 - Apply text-only zoom to system fonts. r=dbaron

One-liner fix for a Stylo regression which causes text-only zoom to not apply to system fonts, which can be an a11y issue. Includes new automated test coverage. Approved for 68.1esr.

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-esr68/rev/32bae0130592
https://hg.mozilla.org/releases/mozilla-esr68/rev/b4a0946bc2db
https://hg.mozilla.org/releases/mozilla-esr68/rev/db86d910c6a2