1561135 - Enabling automaticFontSizeAdjustment somehow enables font inflation

Status: RESOLVED FIXED

(GeckoView :: General, defect, P2)

Description

[James Willcox (:snorp) ([e-mail]) (he/him)]

Fenix enables automaticFontSizeAdjustment, but somehow font inflation is turned on instead (or in addition). You can easily reproduce this by turning on automaticFontSizeAdjustment in GVE and navigate to https://searchfox.org/mozilla-central/source/docshell/test/browser/browser_csp_uir.js

Comment 1

[Jan Henning [:JanH]]

I intentionally did it that way to mimic the way "Use system font size" behaves in Fennec, and specifically regarding Searchfox, it looks like bug 1552781 would fix things there (and presumably on a few other pages, too).

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Jan Henning [:JanH] from comment #1)

I intentionally did it that way to mimic the way "Use system font size" behaves in Fennec, and specifically regarding Searchfox, it looks like bug 1552781 would fix things there (and presumably on a few other pages, too).

The docs for setAutomaticFontSizeAdjustment() don't state this, and the docs for setFontInflationEnabled() specifically says it can't be enabled alongside automatic font size adjustment, so the only reasonable conclusion would be that we don't enable font inflation unless enabled via setFontInflationEnabled(). I would like to keep the automatic size adjustment limited to scaling by the factor set in system settings.

Comment 3

[Dylan Roeh (:droeh) (he/him)]

Attachment: Bug 1561135 - Don't automatically enable font inflation when automatic font size adjustment is enabled in GV. r=snorp!

Comment 4

[Jan Henning [:JanH]]

(In reply to James Willcox (:snorp) ([e-mail]) (he/him) from comment #2)

The docs for setAutomaticFontSizeAdjustment() don't state this,

I plead implementation detail? But true, I didn't explicitly mention this...

and the docs for setFontInflationEnabled() specifically says it can't be enabled alongside automatic font size adjustment, so the only reasonable conclusion would be that we don't enable font inflation unless enabled via setFontInflationEnabled().

Erm, the docs for setFontSizeFactor() say the same thing, so by that logic setAutomaticFontSizeAdjustment() shouldn't actually do anything at all.
The intention behind that was simply that you shouldn't manually manipulate font size and font inflation settings while auto mode is enabled, nothing more.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3ec2481cd32d
Don't automatically enable font inflation when automatic font size adjustment is enabled in GV. r=snorp

Comment 6

[Andreea Pavel [:apavel]]

Backed out for android build bustages

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=253810337&resultStatus=testfailed%2Cbusted%2Cexception&revision=3ec2481cd32dc0e6a9a49ad6ba206edc1251592a

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=253810337&repo=autoland&lineNumber=35758

Backout: https://hg.mozilla.org/integration/autoland/rev/b318af7a75a34407542d0061715ece359742347a

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d2f45bbae09d
Don't automatically enable font inflation when automatic font size adjustment is enabled in GV. r=snorp

Comment 8

[Chris Peterson [:cpeterson]]

Will Fenix need to change anything on their side?

68=wontfix because we don't need to uplift this fix.

Comment 9

[Arthur Iakab [arthur_iakab]]

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

Comment 10

[Dylan Roeh (:droeh) (he/him)]

(In reply to Chris Peterson [:cpeterson] from comment #8)

Will Fenix need to change anything on their side?

If they want to maintain their current behavior, they'll need to enable font inflation anytime they enable automaticFontSizeAdjustment now -- but my understanding is that they were specifically looking not to have font inflation enabled with automaticFontSizeAdjustment, in which case they should be fine leaving things alone.