Closed Bug 1685756 Opened 5 years ago Closed 5 years ago

Too little font inflation on certain devices (e.g. Pixel XL, Pixel 5)

Categories

(Core :: Layout: Text and Fonts, defect)

Product:
Core
Component:
Layout: Text and Fonts
Platform:
All
Android
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
93 Branch
Tracking Flags:
Tracking Status
firefox93 --- fixed

People

(Reporter: JanH, Assigned: JanH)

References

Details

Attachments

(9 files)

gecko-bugzilla.log
10.56 KB, text/plain
Details
gecko-metro.log
26.96 KB, text/plain
Details
gecko-slashdot.log
8.85 KB, text/plain
Details
gecko-swallow.log
130.64 KB, text/plain
Details
fenix_bugzilla.org.png
164.50 KB, image/png
Details
A typical case where minimum-scale size is applied
367 bytes, text/html
Details
Bug 1685756 - Part 1: Use content viewer size for determining font inflation state. r?#layout-reviewers
48 bytes, text/x-phabricator-request
Details | Review
Bug 1685756 - Part 2: Add reftests for explicitly sized viewports. r?#layout-reviewers
48 bytes, text/x-phabricator-request
Details | Review
Bug 1685756 - Part 3: Take default scale into account when disabling font inflation. r?#layout-reviewers
48 bytes, text/x-phabricator-request
Details | Review

It seems that on some phones with certain combinations of screen dimension and densitiy, font inflation presumably ends up believing that the physical screen size is larger than it really is or something along those lines, so the text on non-mobile-friendly pages isn't inflated enough to actually be readable.

According to https://github.com/mozilla-mobile/fenix/issues/17367, this happens e.g. on a Pixel 5 with the following screen dimensions:
1080 x 2160 physical pixels
392 x 785 dp
435 x 434 dpi
DPI category 440 dpi
https://whatismyviewport.com/ reports 396 x 732 px for the viewport and a DPR of ~2.72

On my phone on the other hand font inflation is working fine, with the following display dimensions:
720 x 1184 px
360 x 592 dp
295 x 296 dpi
DPI category 320 dpi
Browser viewport 360 x 512 px and a DPR of 2.0

In both cases font.size.inflation.minTwips is set to 120, i.e. the default value if automatic font size adjustment is enabled in Fenix and the Android system font scale is 1.0.

Is there any way to adjust these parameters, e.g. via about:config? It would be very handy to determine what circumstances are causing this issue.

Not that I can remember. You could try bumping up font.size.inflation.minTwips and see whether at least a high enough value (note that Fenix's automatic font size adjustment may eventually reset that value if it's active) provides a reasonable amount of font inflation, but I don't think that that alone would help much in finding out what exactly is going wrong.

A good starting point for any investigation might be around https://searchfox.org/mozilla-central/rev/c59d9181cbcd8356ce9271723e31be11641e7010/layout/base/nsLayoutUtils.cpp#7541-7571.

Unfortunately it seems that just setting up an emulator with the display metrics of a Pixel 5 (1080x2340, 5.95" screen size, 440 dpi density) isn't enough to reproduce the screenshots in the Github issue.
(Edit: And neither does using the Pixel XL config already included with Android Studio help here, either)

I wonder if that might be worth reporting to the Android Studio people as a bug.

For that you'd still need know what is actually causing the difference between the behaviour on a real phone and the emulator.

Ideally somebody who has such a phone available should look into this, though otherwise I can try adding some debug logging in promising places and we can try working our way towards the issue that way.

Given the device-specific nature of this, it sounds like the point within the function you mention in comment 2 that's doing something wrong is most likely ScreenSizeInchesForFontInflation, and in turn, the information within that that's likely to vary between devices in ways that wouldn't be noticed in many other contexts seems likely to be AppUnitsPerPhysicalInch, which I think might indicate something went wrong in GetDPI, most likely this part.

I have an old OnePlus One I can test with as well as both a Pixel 5 and Pixel XL. Happy to give any debug versions etc. a try.

If you could try this GeckoView example app build, which has had some logging added and font inflation enabled, and check whether you can reproduce the issue with it, too?

If yes, could you look in the logcat output of it, filter for "DPI:" and post the output here?
Also in that case, does manually setting layout.css.dpi in about:config to something like around 440 produce a more reasonable output on those pages?

Flags: needinfo?(paul)

Seems to be the same. The layout.css.dpi setting doesn't seem to make any difference at all.

How do I get the logs?

Flags: needinfo?(paul)

Access your phone's developer options, enable USB debugging, get ADB from here and then open a command line/terminal window. Launch the example app and load a page and then run
adb logcat -dv threadtime | findstr "DPI:" > gecko.log on Windows, respectively
adb logcat -dv threadtime | grep "DPI:" > gecko.log on Linux/Mac.

Don't forget to reset layout.css.dpi back to its default value first.

Attached file gecko-bugzilla.log —
Attached file gecko-metro.log —
Attached file gecko-slashdot.log —
Attached file gecko-swallow.log —

Hmm, according to the first log, the values we get from Android (note the two lines from GeckoAppShell) for screen density and DPI are entirely reasonable (440 dpi and a density of 2.75) and also match what I'm getting on the emulator with the corresponding screen size setting.

The bit further down where we apparently don't have a widget and are falling back to the default value of 96 dpi is a bit weird, but

  • I've just encountered something similar on the emulator without ill effects, and
  • it doesn't happen during the other page loads, yet this bug still happens
  • (and manually overriding layout.css.dpi apparently didn't help either)

I guess I need to take a look at the other parts of the font inflation calculations, then…

Severity: -- → S3

Is there anything I can do to help with this? The fact that it doesn't happen in the emulator is a bit of a pain. I have not done any Android debugging before, if I install Android Studio can I compile and single step through the code?

Sorry for lettings this slide somewhat, but I haven't found much time for this recently.
Regarding your question: Android Studio isn't necessary for compiling itself, but can indeed be used for debugging, including in principle native code (i.e. the Gecko internals) as well.

Unfortunately things seem to get somewhat more cumbersome when having to debug child processes, though, which I have no real experience with because last time I needed to debug something you could still easily switch the geckoview_example app into single process mode, which I gather is no longer possible.
There is some documentation here, or you can try asking in #geckoview on Matrix.
Worst case you could always fall back to printf-style debugging and watching the logcat – see my logging patch above on how to do that.

Thanks Jan. I will try to find some time and give it a go. I'm mostly doing embedded code anyway so I'm used to just printing debug info at least.

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

Unfortunately it seems that just setting up an emulator with the display metrics of a Pixel 5 (1080x2340, 5.95" screen size, 440 dpi density) isn't enough to reproduce the screenshots in the Github issue.
(Edit: And neither does using the Pixel XL config already included with Android Studio help here, either)

I just tested it with Android Studio 4.2.2 and the provided Pixel XL API 30 emulated device and can see the problem.

whatismyviewport.com reports 408x598pix, DPR 3.53, screen size 408x725px.

I tried all the sites I listed at https://github.com/mozilla-mobile/fenix/issues/17367 and they rendered incorrectly. I'm starting to debug now but it seems that your Android Studio setup is different to mine somehow. What API version were you using with the emulated Pixel XL?

I was using an API 28 image. I've now updated to API 30 and Android Studio 4.2.2 and created a new AVD based on the included Pixel XL config (am getting the same values on whatismyviewport.com) and for me it's still working fine (albeit with the old build from the beginning of the year, as updating that would have taken too much time right now).

If you're using the example app, you did enable font inflation there, did you? So you don't have to re-enable it manually in about:config each time you start the app (the enabling pref is controlled by GeckoView, so will be reset each time), add

.automaticFontSizeAdjustment(true)
.fontInflation(true);

to this line here.

This was Fenix, building GeckoView on Windows is not supported. I'll look at it or maybe try in a Linux VM. Can I build it just from a zip snapshot of Mozilla Central or do I have to sign up?

It's a shame I can't tinker with Fenix but it appears you can't debug or even view the Gecko code in Android Studio, unless I'm missing something (very possible).

Attached image fenix_bugzilla.org.png —

Ah okay, with Fenix font inflation should be enabled by default in a fresh install (if you want to double-check – Settings -> Accessibility -> Automatic font sizing should be enabled). I also tried a current Fenix Nightly, too, and as per attached screenshot for me it's looking as expected on the Pixel XL emulator.

(In reply to kuro68k from comment #22)

Can I build it just from a zip snapshot of Mozilla Central or do I have to sign up?

Sure, although what do you mean by "signing up"? For read access, the source code repository is freely accessible.

It's a shame I can't tinker with Fenix but it appears you can't debug or even view the Gecko code in Android Studio, unless I'm missing something (very possible).

The split between Fenix and GeckoView probably makes things a bit more cumbersome, but for debugging this particular issue, the GeckoView example app included in mozilla-central should be fine.

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

Ah okay, with Fenix font inflation should be enabled by default in a fresh install (if you want to double-check – Settings -> Accessibility -> Automatic font sizing should be enabled). I also tried a current Fenix Nightly, too, and as per attached screenshot for me it's looking as expected on the Pixel XL emulator.

The text in your screenshot is still rather small. On a real Pixel XL with Firefox Nightly it is readable but way below the system font size and minimum comfortable readable size.

It seems to have improved a bit but can you please try slashdot.org non-mobile version? It's the best test I have found and is broken in the emulator:

https://i.imgur.com/8RqI2jf.png

(In reply to kuro68k from comment #22)

Can I build it just from a zip snapshot of Mozilla Central or do I have to sign up?

Sure, although what do you mean by "signing up"? For read access, the source code repository is freely accessible.

Yeah, the instructions I found were for contributing, anyway I'm trying to built it under Debian now and regretting my choice of distro. I'll post back if/when I get it working. Trying to get cinnabar installed ATM, thinking of giving up and trying a different distro as Gnome doesn't seem to cope well with a 4k monitor.

I am guessing the change for bug 1718012 might fix this issue.

With the change disable-fontinfl-on-mobile.html fails, but to me the test, disable-fontinfl-on-mobile.html should be inflated because there's no meta viewport tag, thus the document is laid out as a 800px width content and the specified font size in the test document is 12px, it's pretty small compared to 800px, isn't it?

On my local built GeckoViewExample with setting font.size.inflation.minTwips=120, some of the sites in https://github.com/mozilla-mobile/fenix/issues/17367 look much better.

kuro68k, would you mind trying to apply this changeset, https://hg.mozilla.org/try/rev/5d6b279f4b7ccb5ad8f4bf254c936cbb82c18c18, and see if it will result what you expect?

Jan, what do you think about the disable-fontinfl-on-mobile.html? Comments in the test are pretty confusing, I don't quite understand what the test originally aimed.

Flags: needinfo?(paul)
Flags: needinfo?(jh+bugzilla)

(In reply to Hiroyuki Ikezoe (:hiro) from comment #25)

With the change disable-fontinfl-on-mobile.html fails, but to me the test, disable-fontinfl-on-mobile.html should be inflated because there's no meta viewport tag, thus the document is laid out as a 800px width content and the specified font size in the test document is 12px, it's pretty small compared to 800px, isn't it?

disable-fontinfl-on-mobile.html doesn't have a meta viewport tag, but I think the doctype definition is supposed to have the equivalent effect:
<!DOCTYPE html PUBLIC "-//WAPFORUM//DTD XHTML Mobile 1.0//EN" "http://www.wapforum.org/DTD/xhtml-mobile10.dtd">

Edit: Bug 706198 should have the history for that.

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

(In reply to Hiroyuki Ikezoe (:hiro) from comment #25)

With the change disable-fontinfl-on-mobile.html fails, but to me the test, disable-fontinfl-on-mobile.html should be inflated because there's no meta viewport tag, thus the document is laid out as a 800px width content and the specified font size in the test document is 12px, it's pretty small compared to 800px, isn't it?

disable-fontinfl-on-mobile.html doesn't have a meta viewport tag, but I think the doctype definition is supposed to have the equivalent effect:
<!DOCTYPE html PUBLIC "-//WAPFORUM//DTD XHTML Mobile 1.0//EN" "http://www.wapforum.org/DTD/xhtml-mobile10.dtd">

Oh okay, thanks! That means the change regresses the test. :(

One other thing: After some conversation with kuro68k and comparing screenshots, it seems that a simple page like https://www.hq.nasa.gov/office/pao/History/SP-4204/contents.html seems to be inflated correctly, and the font size differences observed between our phones can be explained by the differences in DPI:
Both our phones have a screen approximatively 6.2 cm wide, but my Moto G4 Play has 720 px and a system DPI of 320, resulting in 360 dp, whereas kuro68k's Pixel 5 has 1080 px (50 % more), but a system DPI of 440 (only 37,5 % more) and therefore 392 dp within the same physical screen wdith. This difference is ~8 % and corresponds to the font size difference seen in screenshots of that page, because the Pixel 5 squeezes ~8 % more density-independent pixels (dp) into the same screen width.

With the desktop version of Slashdot on the other hand, the font size difference between our phones is much larger than that (i.e. on the Pixel 5 the text appears something like almost 50 % smaller, not just the 8 % expected by the difference in dp-to-physical-screen-width ratio).
Additionally, I've been finally able to reproduce these issues with the desktop version of Slashdot on an emulator set up for a Pixel 5, too (6" screen size, 1080 x 2340 px, 440 dpi), although I haven't yet had a chance to track things further down.

(In reply to Hiroyuki Ikezoe (:hiro) from comment #25)

kuro68k, would you mind trying to apply this changeset, https://hg.mozilla.org/try/rev/5d6b279f4b7ccb5ad8f4bf254c936cbb82c18c18, and see if it will result what you expect?

Would be happy to, but I can't get Moz Central to build at the moment. I've done clean installs of a couple of Linux distros (Debian and Manjaro) and tried checking the whole lot out and building it, but always get build errors. I'll keep working on it.

I'll keep trying but if you post an APK (or email it to me) I'd be happy to give that a go.

Flags: needinfo?(paul)

I think I've finally got this mostly figured out now: It seems that with a Pixel 5 (or similar phones), font inflation is in fact completely disabled on the desktop Slashdot page, and the reason is this check here:
https://searchfox.org/mozilla-central/rev/f090aabc96ab14cf7a58a15c7e9fdfab384a4ee5/layout/base/PresShell.cpp#11415

The idea of that check was that pages which are displayed at a scale of 1:1 (or perhaps even zoomed in!) by default certainly wouldn't require font inflation.

The problem however is that the zoom values as returned through the nsViewportInfo are being expressed in terms of CSSToScreenScale, i.e. the ratio between physical screen pixels and CSS pixels, and as such on a high-DPI screen this value will be > 1 even at the conceptual 100 % zoom level due to the device scale.

Now the reason why this affects only some pages (like Slashdot or old.reddit.com), but not others (like bugzilla.org, or https://www.hq.nasa.gov/office/pao/History/SP-4204/contents.html), and even then only on some phones, is that the former pages specify a custom viewport width via the "meta viewport" tag, whereas the latter don't. It turns out that when a page doesn't explicitly specify an initial scale, but does specify a width through the "meta viewport" tag, our viewport calculations then calculate a sensible default zoom value for the ViewportInfo.

With Slashdot e.g. specifying a viewport width of 1000 px, this then means that that vInf.GetDefaultZoom() returns 0.72 on my Moto G4 Play (720 px screen divided by 1000 px viewport), but 1.08 on a Pixel 5 (1080 screen px / 1000 viewport px), so in the latter case font inflation is consequently turned off.

With a page that specifies neither an initial scale nor a width in its viewport tag on the other hand, the zoom calculated in Document::GetViewportInfo remains zero, which is then clamped to the minimum zoom value through the nsViewportInfo constructor. Because the minimum zoom level seems to be 25 %, this means that even after multiplication with the device scale the value will likely remain < 1, since only a very-high-DPI device would have a device scale > 4. (The emulated Pixel 5 returns 2.72727275 for example). Pages without a "meta viewport" tag at all will therefore be inflated on all phones.

Hence this bug appears only for pages that are explicitly specifying a viewport width through their "meta viewport" tag, and only when they are being viewed on a phone with more physical pixels than that viewport width.

So fix #1 should be fixing the zoom level comparison in PresShell::DetermineFontSizeInflationState() to take the device scale into account, and #2 is possibly adjusting the viewport calculations so we always calculate a sensible default zoom level for the viewportInfo if the page hasn't explicitly specified an initial scale. I'm not yet sure whether the latter change might or might not have some unwanted side effects, though.

As far as I can tell, that zoom recalculation code dates back to the dawn of font inflation, when the viewport calculations where much simpler. Given that these days there seem to be all sorts of sophisticated calculations for getting a sensible width value even in the absence of an explicitly provided width, maybe it might make sense to just always recalculate the default zoom if !mValidScaleFloat??

Assignee: nobody → jh+bugzilla
Flags: needinfo?(jh+bugzilla)

Does this change https://hg.mozilla.org/try/rev/16e1f0b9811569a71a7d4d9a3403bae545f85393 affect on this issue? I had noticed it when I commented comment 25. Anyway the right thing to do is using MobileViewportManager to get the correct zoom value. That said, the change isn't perfect since it does just work for the top level content document, it doesn't work for iframes. And also there's a timing specific issue, MobileVIewportManager changes its state in response to meta viewport changes, layout size changes after reflows, etc. etc. So I assume we might have to call DetermineFontSizeInflationState whenever the MobileViewportManager's state changes.

Fantastic work Jan. You seem to have identified the issue.

FWIW even Chrome doesn't always get it right: https://imgur.com/9uFGXnm

Anyway I wanted to add one other thing. This bug affects extensions too. The mobile extension UIs are tiny on my Pixel 5 too, e.g. the translate web pages extension:

https://imgur.com/ntfJ0x3 tiny
https://imgur.com/UnJHoLo fine

Seems to depend on the underlying web page!

Hopefully the fix will fix both.

Bug 1208023 and https://hg.mozilla.org/mozilla-central/rev/1685ea1f5068708540e0ac62a6f206defbd29afb have some more history regarding the evolution of the default zoom value's meaning in the viewport info. Based on that, I think it might indeed be safe to change the calculations, and I just need to update a few test expectations accordingly.

Unfortunately this also breaks a lot of the font inflation reftests, though, so I need to check what is happening there (i.e. did those tests all just mistakenly rely on the current behaviour, or is there a genuine problem with my initial idea?).

(In reply to Hiroyuki Ikezoe (:hiro) from comment #31)

Does this change https://hg.mozilla.org/try/rev/16e1f0b9811569a71a7d4d9a3403bae545f85393 affect on this issue? I had noticed it when I commented comment 25.

I didn't yet check the effect of your patches on all the other pages from https://github.com/mozilla-mobile/fenix/issues/17367 (since you mentioned that they might "look much better."), but for this viewport-tag-related issue affecting e.g. Slashdot in particular it doesn't make any difference, i.e. the default zoom check still needs to be fixed even after moving the code into the MVM.

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

(In reply to Hiroyuki Ikezoe (:hiro) from comment #31)

Does this change https://hg.mozilla.org/try/rev/16e1f0b9811569a71a7d4d9a3403bae545f85393 affect on this issue? I had noticed it when I commented comment 25.

I didn't yet check the effect of your patches on all the other pages from https://github.com/mozilla-mobile/fenix/issues/17367 (since you mentioned that they might "look much better."), but for this viewport-tag-related issue affecting e.g. Slashdot in particular it doesn't make any difference, i.e. the default zoom check still needs to be fixed even after moving the code into the MVM.

Sorry for the confusion. that was my wrong impression.

Anyways, I'd really be opposed tweaking (specifically dropping the !mWidthStrEmpty check) in GetViewportInfo. We've been doing much efferts on the stuff to match our behavior to Chrome's (and Safari). If you want to do something specifically for font inflation stuff without using MobileViewportManager, please add another function for the purpose.

I mean, if you want to detect font inflation based on the device screen size, it would be definitely better to add another function to calculate the default zoom (I think the default zoom value based on the screen size will be different from the normal one).

Given that at that point I have both the screen/display size and the viewport width available, I suppose I could just do the calculation locally myself if the default zoom provided by the viewport isn't valid, but is the default zoom value in the viewport info really contractual?

Looking through Searchfox, to me it doesn't seem like the viewport info is actually directly web-observable, and as per bug 1208023 the MVM will recalculate the actual zoom (which would indeed be web-observable) anyway when IsDefaultZoomValid() == false. So the only things that care about the actual value of the default zoom when !IsDefaultZoomValid() seem to be a) a few viewport tests and b) the font inflation code.

To be honest, I haven't considered about font inflation stuff carefully, FWIW, there are unintuitive auto-scale down contents, attaching file is an example.

I think in this case font inflation should happen, no?

No, I don't think so. Several things:

  1. "width=device-width" means that font inflation should be turned off (through the IsAutoSizeEnabled() check), because font inflation is only intended for pages not written with mobile devices in mind.
  2. Your example page doesn't have enough text to trigger font inflation anyway.
  3. Because the page has a width string, the default zoom recalculation code in Document::GetViewportInfo does run even today and sets the default zoom based on the viewport and screen widths.
  4. Because of !IsDefaultZoomValid(), the MVM then does its own thing anyway and zooms out the page for shrink-to-fit, albeit bounded by the minimum scale (so if I make that page 800 % wide, it will still only be zoomed out to 25 %, because that's the minimum scale).

I've realised though that using the screen size only really works for mobile, where assuming screen width ≈ browser width is usually valid (or at least used to be, given split-screen and picture-in-picture mode on newer Android versions).

On a desktop (or apparently even within the (ref)test environment on mobile) this assumption of course breaks down, because it's actually the width of the content area that we're interested in (see also the comment above). So I guess I'll leave that alone for now and just fix the comparison logic to take the device scale into account.

@Hiro: I think I might pinch your patch from https://hg.mozilla.org/try/rev/16e1f0b9811569a71a7d4d9a3403bae545f85393 after all, because it turns out that writing a sane reftest for this doesn't really work if the result depends on the screen size of the machine executing the test.

Edit: Except as you said it doesn't work for iframes etc., so needs some more work...

So I can just go up the chain until reaching the top-level document and use that, except it looks like that won't always work with Fission (grrrr!!).

Then again, I'm not sure how much the viewport-width calculations for regarding font inflation really make sense for iframes anyway, given that a document loaded there will behave quite differently from a top-level document (width depends on the iframe and not on the viewport and no zoom-to-fit behaviour), but that's probably a separate matter (bug 1724311).

I guess for the time being I'll just try getting the top-level content viewer size if possible and otherwise just fall back to using whatever content viewer size is obtainable within the current process. For no viewport tag at all and "width=device-width" this shouldn't result in any behaviour changes. For some combinations of explicitly specified viewport size and frame width this might lead to different behaviour if fission is on, but hopefully that combination should be exotic enough to not really matter.

Besides, apart from testing and debugging this only really matters in GeckoView, where fission still is some way off, and maybe there's a chance this could be resolved via bug 1724311 then.

See Also: → 1724608
See Also: → 1724623

Tests are looking good both with the expected failures (the one failure was because I hadn't yet added the assert count declaration for one of the two tests requiring them) and after the fix.

For pages with an explicitly sized viewport, whether or not we enable font
inflation depends on whether the viewport is larger than content viewer size (so
it will be displayed zoomed out) or not.

For historical reasons, so far we've used the screen size as a proxy for the
content viewer size. On a phone this is a reasonable approximation (albeit a bit
less so now that Android also offers split-screen and picture-in-picture modes),
but when testing/debugging on a desktop computer, this means that the results
will depend on the screen size of the machine in question, which makes it rather
hard writing sensible test cases for that scenario.

Therefore, we're finally follow up with that TODO comment in the existing code
and start using the content viewer size of the top-level document instead.
Fission means that approach won't easily work for cross-process iframes, but
given that the current calculations don't make much sense for frames anyway, we
just accept that limitation, since a proper solution as per bug 1724311 would
obviate any work done here anyway.

Touching layout.css.devPixelsPerPx during a reftest seems to cause a few
peculiarities:

  • On OS X, increasing layout.css.devPixelsPerPx almost immediately runs into
    bug 1263092, so I'm skipping that test there.
  • Likewise, under Webrender on Android using a value > 1 causes some weird
    display corruption issues during all subsequent tests running within the same
    task (bug 1724608).
  • We're randomly hitting the "can't mark frame dirty during reflow" assertion
    again (bug 1724623).

Font inflation isn't required for pages whose viewport is narrow enough that we
don't have to zoom out when doing zoom-to-fit.

As the viewport scale is given as a CSSToScreenScale, this means that what we're
actually doing at the moment, though, is comparing the width of the viewport in
CSS pixels to the width of the screen in physical pixels, without taking the
default scale on a high-DPI device into account.

This issue affects neither pages using a "width=device-width" viewport (the
IsAutoSizeEnabled() check handles those), nor pages without an explicitly
specified viewport at all (the viewport info returns the minimum zoom level in
that case, i.e. 25 %, which would therefore require a device scale of > 4 in
order for us to erroneously turn off font inflation - even modern phones don't
seem to quite reach that sort of pixel density, e.g. a Pixel 5 only reports a
default scale of ~2.73.

For pages with an explicitly sized viewport this can however mean that we turn
off font inflation too early: Slashdot e.g. uses "width=1000", which means that
on a phone with a 720 px screen font inflation is correctly enabled, but on a
different phone squeezing 1080 px into the same physical screen width (i.e.
higher DPI, but not a physically larger screen), font inflation is suddenly
and incorrectly turned off.

Pushed by mozilla@buttercookie.de: https://hg.mozilla.org/integration/autoland/rev/bb38f12dc2a8 Part 1: Use content viewer size for determining font inflation state. r=hiro https://hg.mozilla.org/integration/autoland/rev/194be1464b3d Part 2: Add reftests for explicitly sized viewports. r=hiro https://hg.mozilla.org/integration/autoland/rev/7414cb0c0cac Part 3: Take default scale into account when disabling font inflation. r=hiro

Nightly just updated to 93 and I can confirm that this is fixed on a real Pixel 5 and Pixel XL! Thank you do much for working on this, Firefox is finally usable for me now. Jan, your hard work is really appreciated, and Hiroyuki too.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: