Open
Bug 1493266
Opened 6 years ago
Updated 9 months ago
Follow up to bug 1428670 - ensure correct font inflation min. font size calculation and reflow of affected frames when value changes
Categories
(Core :: Layout: Text and Fonts, enhancement, P5)
Core
Layout: Text and Fonts
Tracking
()
NEW
People
(Reporter: JanH, Unassigned, NeedInfo)
References
Details
Attachments
(1 obsolete file)
If any of the input factors into the font inflation minimum font size calculation causes a change in the font inflation minimum font size, all affected frames need to be reflowed again so that the layout matches the new font size.
Some factors are considered during reflow as part of refreshing the FontInflationData for each font inflation flow root - any changes there are detected by comparison with the previous FontInflationData and cause the whole affected subtree to be marked as dirty.
Even after bug 1428670 though, the screen size is only taken into account dynamically when actually accessing the minimum font size through nsLayoutUtils [1]. While in practice it is likely that a screen size change will trigger a reflow covering all frames affected by the font inflation change anyway, in theory this means that a change in the screen size could cause font inflation's minimum font size to change without forcing a reflow of all affected frames.
A possible solution might be to continue what I started in bug 1428670 and move the remaining bits of nsLayoutUtils::MinimumFontSizeFor into the FontInflationData as well, which would also avoid repeatedly recalculating the same minTwips based minimum font size [2].
While this ensures that any change in the minimum font size will be detected and cause a reflow of the affected frames, so that the layout and the rendered font size will always stay in sync, it might cause a different problem in that changes in screen size or visible area will, if I'm not mistaken, only be picked up if the respective font inflation root is actually reflowed, which causes a refresh of the FontInflationData [3].
Additionally, some of the relevant dimensions for font inflation depend on the writing direction. The calculations in nsLayoutUtils technically always reference the writing direction of the concrete frame in question, however the calculations in the FontInflationData already assume that all frames covered by the respective font inflation flow root have the same writing direction. We should check that this is actually the case, i.e. that a change in writing direction actually forces that frame to be a new font inflation flow root [4].
[1] https://dxr.mozilla.org/mozilla-central/rev/ebeba937ca2a01df86d98089f2368bb975182dcc/layout/base/nsLayoutUtils.cpp#8298-8303
[2] It also used to be the case that retrieving the screen size could be a relatively expensive operation, although since first bug 1257304 and later bug 1194751 in conjunction with bug 1475875 for Android this is no longer the case.
[3] https://dxr.mozilla.org/mozilla-central/rev/ebeba937ca2a01df86d98089f2368bb975182dcc/layout/generic/ReflowInput.cpp#600
[4] If my reasoning in https://phabricator.services.mozilla.com/D5578#inline-25245 is correct, that should already be the case, though.
Flags: needinfo?(dbaron)
I'd note a bunch of the background here is in https://phabricator.services.mozilla.com/D5578
Comment 2•6 years ago
|
||
Marking this as P5 for now, as it sounds like it's unlikely to be causing serious issues in practice at this point, though improving (and cleaning up/documenting) the code further would still be welcome.
Priority: -- → P5
Updated•2 years ago
|
Severity: normal → S3
Updated•9 months ago
|
Attachment #9382872 -
Attachment is obsolete: true
You need to log in
before you can comment on or make changes to this bug.
Description
•