1857158 - CSSPropertyRule.name should not serialize @property name as identifier

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Frédéric Wang (:fredw)]

This is the remaining failure for /css/css-properties-values-api/at-property-cssom.html

See https://chromium-review.googlesource.com/c/chromium/src/+/2238117
'The serialization must follow the "serialize an identifier" algorithm,
otherwise characters like U+0009 CHARACTER TABULATION will not be
escaped correctly.'

Comment 1

[Frédéric Wang (:fredw)]

OK, so I checked a bit that, here is the spec: https://drafts.css-houdini.org/css-properties-values-api/#the-css-property-rule-interface

No escaping is mentioned when accessing CSSPropertyRule.name directly, but when serializing the whole CSSPropertyRule it is explicitly said that the name is serialized as an identifier.

The test assumes that no escaping is done for CSSPropertyRule.name but that escaping is done when serializing the whole CSSPropertyRule. Chrome and WebKit implement that behavior and so pass the test:
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/css/css_property_rule.cc
https://searchfox.org/wubkat/source/Source/WebCore/css/CSSPropertyRule.cpp

Currently we are failing the test because in both cases we perform escaping during serialization (via serialize_atom_name):
https://searchfox.org/mozilla-central/rev/d436104fc6c31677b08a851796bead25153be699/servo/components/style/properties_and_values/rule.rs#290
https://searchfox.org/mozilla-central/rev/d436104fc6c31677b08a851796bead25153be699/servo/components/style/properties_and_values/rule.rs#313

@Emilio: What do you think about that? Does Chrome/WebKit's behavior seem correct and we should align on it?

Comment 2

[Emilio Cobos Álvarez (:emilio)]

@Emilio: What do you think about that? Does Chrome/WebKit's behavior seem correct and we should align on it?

Seems reasonable, it is what we do for keyframe names too here

We shouldn't change the ToCss implementation, we should just make PropertyRule_GetName not use it and return the atom directly.

Comment 3

[Frédéric Wang (:fredw)]

Attachment: Bug 1857158 - CSSPropertyRule.name should not serialize @property name as identifier. r=emilio

The spec just says to return the name as is [1]. This aligns with
WebKit and Chrome's behavior and fixes remaining issue in [2].

[1] https://drafts.css-houdini.org/css-properties-values-api/#the-css-property-rule-interface
[2] /css/css-properties-values-api/at-property-cssom.html

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/812ec42fdd13
CSSPropertyRule.name should not serialize @property name as identifier. r=emilio

Comment 5

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/812ec42fdd13