Closed Bug 1561135 Opened 5 months ago Closed 5 months ago

Enabling automaticFontSizeAdjustment somehow enables font inflation

Categories

(GeckoView :: General, defect, P2)

Unspecified
All
defect

Tracking

(firefox68 wontfix, firefox69 fixed)

RESOLVED FIXED
mozilla69
Tracking Status
firefox68 --- wontfix
firefox69 --- fixed

People

(Reporter: snorp, Assigned: droeh)

Details

Attachments

(1 file)

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

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).

(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.

Assignee: nobody → droeh

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (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.

Attachment #9073861 - Attachment description: Bug 1561135 - Disable font inflation when automatic font size adjustment is enabled in GV. r=snorp! → Bug 1561135 - Don't automatically enable font inflation when automatic font size adjustment is enabled in GV. r=snorp!
Pushed by droeh@mozilla.com:
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
Pushed by droeh@mozilla.com:
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

Will Fenix need to change anything on their side?

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

Priority: -- → P2
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

(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.

Flags: needinfo?(droeh)
You need to log in before you can comment on or make changes to this bug.