1468806 - Font editor field for "size" causes unexpectedly huge fonts on small tweaks, for elements with inherited "em" font-size

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P3)

Description

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

STR:
 1. turn on devtools.inspector.fonteditor.enabled
 2. View attached testcase.
 3. Right-click the underlined text and choose "Inspect", and click the Fonts panel.
 4. For the Size field in the font editor, click the up arrow and then down arrow (to make and then "undo" a minor tweak)


ACTUAL RESULTS:
The text suddenly gets ~5x as big and stays that way (because we suddenly apply approximately "font-size: 5em" to the inline style for *this specific element* -- i.e. we say to be 5x *the parent's font size* -- rather than the initial rendering, where we were *inheriting* whatever font-size:5em computed to *on the parent*.)

EXPECTED RESULTS:
Visually: "uparrow" should result in a tiny, incremental change to font size. And "downarrow" should then take you back to the original font size.

Functionally: we should probably have shown the (inherited) computed font-size in px units rather than em units for this element, so that it has the same effect when you make it an explicit inline style attribute and make tweaks to it.

I discovered this in the wild at https://developer.microsoft.com/en-us/microsoft-edge/testdrive/demos/variable-fonts/ (using the word "Fonts" in "Variable Fonts" in the center of the first load page there)

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: reference case 1 (using "font" shorthand)

FWIW, If I change the testcase to use the "font" shorthand to set the font-size on the ancestor (font: 5em serif), then I get "EXPECTED RESULTS" -- the fonts panel shows me the size in px units, which does the right thing when I hit uparrow/downarrow.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: screencast of issue

Comment 3

[Razvan Caliman [:rcaliman]]

Good catch. Thanks for filing! 

This is going to be the tip of a tricky "special cases" iceberg. We want to preserve the authored unit types for declared properties. In this case, the editor mistakenly picks-up the "5em" inherited from the element's parent instead of falling back to the computed style (approx. 80px) for the font-size that's not explicitly declared.

Comment 4

[Razvan Caliman [:rcaliman]]

Attachment: Bug 1468806 - Do not populate Font Editor with inherited property values; use computed.

Review commit: https://reviewboard.mozilla.org/r/252774/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/252774/

Comment 5

[Patrick Brosset <:pbro>]

Comment on attachment 8987536 [details]
Bug 1468806 - Do not populate Font Editor with inherited property values; use computed.

https://reviewboard.mozilla.org/r/252774/#review259548

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/29be8a075df9
Do not populate Font Editor with inherited property values; use computed. r=pbro

Comment 7

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/29be8a075df9

Comment 8

[Cristian Fogel [:cfogel]]

Marking as GFV since it has clear and simple STR.