Closed Bug 1404545 Opened 4 years ago Closed 4 years ago
stylo: support Fennec's "Honor system font sizes" setting
59 bytes, text/x-review-board-request
The Gecko implementation was bug 1328868. /** * This setting corresponds to a global text zoom setting affecting * all content that is not already subject to font size inflation. * It is interpreted as a percentage value that is applied on top * of the document's current text zoom setting. * * The resulting total zoom factor (text zoom * system font scale) * will be limited by zoom.minPercent and maxPercent. */ pref("font.size.systemFontScale", 100);
It looks like we already support this; Stylo already uses mEffectiveTextZoom, and always has. Is there evidence stating that we don't support this?
Makoto, do you have more info on what's not working here?
failed log is here. https://treeherder.mozilla.org/logviewer.html#?job_id=134365073&repo=try&lineNumber=2774 10:39:33 INFO - 188 INFO TEST-UNEXPECTED-FAIL | mobile/android/tests/browser/chrome/test_settings_fontinflation.html | reftest comparison: != fontScaleOn fontScaleOff 10:39:43 INFO - 191 INFO TEST-UNEXPECTED-FAIL | mobile/android/tests/browser/chrome/test_settings_fontinflation.html | reftest comparison: == fontScaleOn cssZoom 10:39:43 INFO - 195 INFO TEST-UNEXPECTED-FAIL | mobile/android/tests/browser/chrome/test_settings_fontinflation.html | reftest comparison: == fontScaleOn textZoom 10:40:15 INFO - 215 INFO TEST-UNEXPECTED-FAIL | mobile/android/tests/browser/chrome/test_settings_fontinflation.html | reftest comparison: != noZoom fontScaleNoFontInflation To build Fennec/sytlo, it requires bug 1397764.
I'm in Montreal for the week and the hashmap stuff will take priority so I probably won't be able to get to this for a while (and i don't yet have an android build). Unassigning for now. I can answer questions about the zoom stuff though. As far as I can tell this should work; because both we and Gecko use mEffectiveTextZoom. Perhaps we don't send the correct change hint?
Assignee: manishearth → nobody
I did setup android build. I can take a look into this.
Assignee: nobody → hikezoe
Status: NEW → ASSIGNED
It seems to me that the android's system font size stuff (or just the failure test case) relies on MediaFeatureValuesChanged call in nsPresContext::UpdateEffectiveTextZoom(). In stylo the call is skipped basically, if I tweaked that MediaFeatureValuesChanged is called for stylo, the failure test passed normally. I am not yet sure whether we should fix stylo or android side, but probably we should fix android implementation (or the test case).
In the test case, test_settings_fontinflation.html, when nsPresContext::UpdateEffectiveTextZoom() is called, stylist.device.default_values has been already created with not-yet-zoomed value even if PresShell::DidInitialized() is false. So in servo case we should regenerate the default values. And to do that, I think calling MediaFeatureValuesChanged() is fine.
Let's see what happens. https://treeherder.mozilla.org/#/jobs?repo=try&revision=c69dbc388be3839f6734075193efdcd4d4d893d5
Comment on attachment 8919136 [details] Bug 1404545 - Force to call MediaFeatureValuesChanged for stylo in UpdateEffectiveTextZoom. https://reviewboard.mozilla.org/r/190050/#review195274
Attachment #8919136 - Flags: review?(cam) → review+
Thank you for the review!
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/1ccba28267ff Force to call MediaFeatureValuesChanged for stylo in UpdateEffectiveTextZoom. r=heycam
You need to log in before you can comment on or make changes to this bug.