1800950 - 'all' shorthand should not include 'size' descriptor

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Oriol Brufau [:Oriol]]

This makes no sense, size is a descriptor for @page, not a property.

var {style} = document.createElement("div");
style.all = "initial";
style.getPropertyValue("size"); // "initial"
style.all = "inherit";
style.getPropertyValue("size"); // "inherit"

Also:

var {style} = document.createElement("div");
style.all = "initial";
style.all; // "initial"
style.cssText = [...style].map(p => p + ": " + style.getPropertyValue(p)).join("; ");
style.all; // ""

since the size descriptor is not indexed (because it's not a property), then style.all is not preserved when copying the styles.

This causes some failures in https://github.com/web-platform-tests/wpt/pull/36984

Comment 1

[Jonathan Kew [:jfkthame]]

The same issue also applies to page, if layout.css.named-pages.enabled is turned on.
[edit: ignore this -- as Oriol points out below, page is indeed a property]

Comment 2

[Oriol Brufau [:Oriol]]

(In reply to Oriol Brufau [:Oriol] from comment #0)

since the size descriptor is not indexed (because it's not a property)

Actually, it's indexed:

style.cssText = "";
[...style].includes("size"); // false
style.all = "initial";
[...style].includes("size"); // true

The problem is that even though we can see it and read its value, can't set it directly using setProperty, but it works via all:

style.getPropertyValue("size"); // "initial"
style.setProperty("size", "inherit");
style.getPropertyValue("size"); // "initial"
style.setProperty("all", "inherit");
style.getPropertyValue("size"); // "inherit"

page doesn't have this problem:

CSS.supports("size", "initial"); // false
CSS.supports("page", "initial"); // true

and looking at the examples in https://drafts.csswg.org/css-page-3/#using-named-pages it seems that page should be a CSS property indeed.

So it's just size which is implemented as a property but it should be a descriptor of @page.

Comment 3

[Oriol Brufau [:Oriol]]

In short, this provides the problematic properties:

var {style} = document.createElement("div");
style.all = "initial";
[...style].filter(p => !CSS.supports(p, style.getPropertyValue(p))); // ["size"]

Comment 4

[Oriol Brufau [:Oriol]]

Seems a matter of just

--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -169,6 +169,8 @@ pub mod shorthands {
                 continue;
             if not p.enabled_in_content() and not p.experimental(engine):
                 continue;
+            if p.rule_types_allowed & RULE_VALUES["Style"] == 0:
+                continue;
             if p.logical:
                 logical_longhands.append(p.name)
             else:

Comment 5

[Oriol Brufau [:Oriol]]

Attachment: Bug 1800950 - Exclude 'size' from the 'all' shorthand. r?emilio

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/76e180dbe144
Exclude 'size' from the 'all' shorthand. r=emilio

Comment 7

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

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

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4052376a0870
Update property database.

Comment 9

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/76e180dbe144
https://hg.mozilla.org/mozilla-central/rev/4052376a0870

Comment 10

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

Upstream PR merged by moz-wptsync-bot