1777902 - System font text no longer has system font sizes when devPixelsPerPx is > 0

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Karl Tomlinson (:karlt)]

The font size returned by nsLookAndFeel::PerThemeData::GetFont() is not longer correcting the returned size (in CSS pixels) for the change in the size of a CSS pixel.

The dependency of system font sizes on "layout.css.devPixelsPerPx" is a regression from https://hg.mozilla.org/mozilla-central/rev/7305249ed4b6#l1.12, but the system font sizes were already wrong since https://hg.mozilla.org/mozilla-central/rev/6e1cbabce0af
The workaround is no longer effective.

STR:

1. Set "layout.css.devPixelsPerPx" pref to something small e.g. "0.6666666667"
2. Restart

Expected: slightly smaller UI but UI text of same size.
Actual: UI text is smaller.

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: Bug 1777902 update layout.css.devPixelsPerPx doc for text scale factor changes r?emilio

as of https://hg.mozilla.org/integration/autoland/rev/6e1cbabce0af

Comment 2

[Karl Tomlinson (:karlt)]

Attachment: Bug 1777902 restore conversion of GDK font sizes to CSS pixels when devPixelsPerPx > 0 r?emilio

GetTextScaleFactor() is used for the CSS pixel conversion because that is the
factor used in determining the size of a CSS pixel and differs from
gfxPlatformGtk::GetFontScaleFactor() when "ui.textScaleFactor" is set.

The previous "Scale the font for the current monitor" comment was confusing in
the presence of the scale factor from monitor zero.
https://hg.mozilla.org/mozilla-central/rev/7305249ed4b6#l1.12
CSS pixels differ in sizes on different monitors so scaling for the current
monitor here has not been performed here since
https://hg.mozilla.org/mozilla-central/rev/56f4c25656d9b5ff4b6fefe187e42cad4125fc0b#l13.51

Depends on D150930

Comment 3

[Emilio Cobos Álvarez (:emilio)]

I'm not sure I agree with the expected behavior fwiw, and other users seem to have the same opinion (bug 1765021, bug 1752748). But given this restores behavior and it's a non-default setting I guess it's fine, as long as we make this work properly without a restart which should be trivial, will comment on the patch.

Comment 4

[Karl Tomlinson (:karlt)]

Thank you for your grace, Emilio. I suspect we can keep both sets of people happy if we have different behavior for "devPixelsPerPx" and "textScaleFactor". These values are usually multiplied together when "os-zoom-behavior" has its default value, and so we have two knobs to turn.

The decision on which should change the size of GTK fonts and widgets is not clear cut. I'll put up a patch for each. Please let me know if you have a preference. Note that neither approach will scale GTK dialogs.

The name and documentation for "devPixelPerPx" imply that it changes the size of only CSS pixels, while "ui.textScaleFactor" might be inferred to mean a scale factor for all text.

However, the current behavior of these prefs wrt scrollbars doesn't really align with the implied behavior wrt CSS pixels: "devPixelPerPx" scales scrollbars while "textScaleFactor" does not. That's largely a quirk of how "devPixelPerPx" usage now happens to more closely align with the GDK monitor scale factor on the primary monitor. Scrollbar sizes in the GTK port are not based on GTK scrollbar sizes, but in the ideal world they would be.

So the behavior of "devPixelPerPx" wrt scrollbars is somewhat consistent with the expectations of those that want to scale everything, while the behavior for textScaleFactor is more consistent with the expectations of those that only want to change the size of a CSS "px".

The other place where I see "devPixelPerPx" and "textScaleFactor" behaving differently is in menus. "devPixelPerPx" scales the line-height, but "textScaleFactor" does not. I don't know the mechanism here. Checkboxes in the menus scale with line-height, but I don't know which is cause or effect: whether the checkboxes fill the line-height or the line-height expands to contain the checkboxes. These are not GTK checkboxes.

"devPixelPerPx" is a float that can represent any fraction (at least close enough so that rounding to app units kills any error), while "textScaleFactor" is an integer percentage, so wouldn't be able to cancel a monitor scale factor of 3, for example.

Comment 5

[Karl Tomlinson (:karlt)]

(In reply to Karl Tomlinson (:karlt) from comment #4)

"devPixelPerPx" is a float that can represent any fraction (at least close enough so that rounding to app units kills any error), while "textScaleFactor" is an integer percentage, so wouldn't be able to cancel a monitor scale factor of 3, for example.

This is more significant on WINNT where monitor scale factors often involve prime numerators, 5/4, 3/2, and 7/4.
That might be the strongest reason to prefer "devPixelPerPx" for those who want to undo the monitor scale factor without shrinking system fonts, get better zoom levels when image viewing with the default value of "toolkit.zoomManager.zoomValues", get better "Default zoom" options in about:preferences, and get a more compact UI. But let me know what you think.

Comment 6

[Karl Tomlinson (:karlt)]

Attachment: Bug 1777902 don't change the size of system fonts when ui.textScaleFactor is set r?emilio

Depends on D150930

Comment 7

[Karl Tomlinson (:karlt)]

(In reply to Karl Tomlinson (:karlt) from comment #4)

The other place where I see "devPixelPerPx" and "textScaleFactor" behaving differently is in menus. "devPixelPerPx" scales the line-height, but "textScaleFactor" does not. I don't know the mechanism here. Checkboxes in the menus scale with line-height, but I don't know which is cause or effect: whether the checkboxes fill the line-height or the line-height expands to contain the checkboxes. These are not GTK checkboxes.

Experiments with small "devPixelPerPx" produce line-heights smaller than check boxes, so it is not the line-height expanding to contain the check boxes.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/990f5db2d5e8
update layout.css.devPixelsPerPx doc for text scale factor changes r=emilio
https://hg.mozilla.org/integration/autoland/rev/68ec3d16cb3f
don't change the size of system fonts when ui.textScaleFactor is set r=emilio

Comment 9

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/990f5db2d5e8
https://hg.mozilla.org/mozilla-central/rev/68ec3d16cb3f

Comment 10

[Karl Tomlinson (:karlt)]

Should be able to test this.

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:karlt, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox103 to wontfix.

For more information, please visit auto_nag documentation.

Comment 12

[Donal Meehan [:dmeehan]]

:emilio could you please take a look at an uplift request, since :karlt is out until July 25th?

Comment 13

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9284280 [details]
Bug 1777902 don't change the size of system fonts when ui.textScaleFactor is set r?emilio

Beta/Release Uplift Approval Request

• User impact if declined: Matches windows font scaling with ui.textScaleFactor pref. Other than docs changes, it's a one-liner than matches Windows. I think it makes sense to uplift to avoid changing behavior in different releases, since 103 will also contain bug 1773342.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): One-liner to account for a pref in font size calculations.
• String changes made/needed: none
• Is Android affected?: No

Comment 14

[Donal Meehan [:dmeehan]]

Comment on attachment 9283973 [details]
Bug 1777902 update layout.css.devPixelsPerPx doc for text scale factor changes r?emilio

Approved for 103.0b9, thanks.

Comment 15

[Donal Meehan [:dmeehan]]

Comment on attachment 9284280 [details]
Bug 1777902 don't change the size of system fonts when ui.textScaleFactor is set r?emilio

Approved for 103.0b9, thanks.

Comment 16

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6625a4800798
https://hg.mozilla.org/releases/mozilla-beta/rev/73e75cf92683

Comment 17

[Karl Tomlinson (:karlt)]

Attachment: Bug 1777902 Test that system font sizing is independent from ui.textScaleFactor r?emilio

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

A patch has been attached on this bug, which was already closed. Filing a separate bug will ensure better tracking. If this was not by mistake and further action is needed, please alert the appropriate party. (Or: if the patch doesn't change behavior -- e.g. landing a test case, or fixing a typo -- then feel free to disregard this message)

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b4314676d8ff
Test that system font sizing is independent from ui.textScaleFactor r=emilio

Comment 20

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/b4314676d8ff