1436031 - [stylo] unexpected serialization of the 'font' shorthand when stylo is enabled

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Jonathan Kew [:jfkthame]]

Testcase:
    data:text/html,<div style="font:bold medium serif">hello

With stylo disabled:
    document.getElementsByTagName('div')[0].style.font
    -> "bold medium serif"

With stylo enabled:
    document.getElementsByTagName('div')[0].style.font
    -> "normal normal bold normal medium serif"

which seems unexpectedly verbose, and inconsistent with how other shorthands are serialized.

FWIW, Chrome agrees with the non-stylo (old Gecko) serialization here.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Yeah... This doesn't make much sense.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1436031: Test the serialization of the font shorthand.

Nothing like fixing bugs by removing code.

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

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8948741 [details]
Bug 1436031: Test the serialization of the font shorthand.

I submitted this directly to Github, since there's no Gecko code at all.

Comment 4

[Xidorn Quan [:xidorn] UTC+10]

We probably should have a test, preferably web-platform test for this case.

jfkthame, mind submitting one (here or to wpt directly)?

Comment 5

[Emilio Cobos Álvarez (:emilio)]

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #4)
> We probably should have a test, preferably web-platform test for this case.
> 
> jfkthame, mind submitting one (here or to wpt directly)?

I didn't include one because shorthand serialization is a mess and not really properly specified... But on hindsight I guess we should, per https://github.com/w3c/csswg-drafts/issues/1564.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

I can write it I guess.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8948741 [details]
Bug 1436031: Test the serialization of the font shorthand.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/218134/diff/1-2/

Comment 8

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8948741 [details]
Bug 1436031: Test the serialization of the font shorthand.

https://reviewboard.mozilla.org/r/218134/#review224016

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Actual fix in https://hg.mozilla.org/integration/autoland/rev/211f446b6a7b.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7806af854343
Test the serialization of the font shorthand. r=xidorn

Comment 11

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/7806af854343

Comment 12

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/w3c/web-platform-tests/pull/9462 for changes under testing/web-platform/tests

Comment 13

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Can't merge web-platform-tests PR due to failing upstream tests

Comment 14

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Can't merge web-platform-tests PR due to failing upstream tests

Comment 15

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Can't merge web-platform-tests PR due to failing upstream tests

Comment 16

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Can't merge web-platform-tests PR due to failing upstream tests

Comment 17

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged