Bug 247741 - cssText can serialize duplicated declarations
Summary: cssText can serialize duplicated declarations
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on: 247734
Blocks:
  Show dependency treegraph
 
Reported: 2022-11-10 09:32 PST by Oriol Brufau
Modified: 2022-11-17 09:33 PST (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Oriol Brufau 2022-11-10 09:32:58 PST
Run this:

    document.body.style.cssText = "margin: var(--m)";
    document.body.style.marginTop = "1px";
    document.body.style.cssText;

Expected: "margin-top: 1px; margin-right: ; margin-bottom: ; margin-left: ;"
Actual: "margin-top: 1px; margin: var(--m);"

margin-top appears twice (once explicitly and once via margin shorthand).
The problem is that margin-top detect that it can't be serialized via shorthand so it appears as a longhand, but then bug 247734 prevents other longhands from realizing it and they serialize via the shorthand, which overrides margin-top.
Note the duplication only happens if the different longhand(s) appear first, otherwise it's just bug 247734 without duplication.

Run this:

    document.body.style.cssText = "grid-area: var(--a); grid-row-start: 1";
    document.body.style.cssText;

Expected: "grid-row-end: ; grid-column-start: ; grid-column-end: ; grid-row-start: 1;"
Actual: "grid-area: var(--a); grid-row-start: 1;"

That's a similar case, but unlike for margin, we get a duplicated grid-row-start even if it's not the 1st one.
The reason is that cssText doesn't try to serialize using the grid-area shorthand, but var() can bypass this.
So the shorthand serializes due to the longhands set to a pending-substitution value, and then grid-row-start doesn't check whether grid-area has already been used.
TBH in this specific case the actual serialization seems better than the expected one (see https://github.com/w3c/csswg-drafts/issues/2515), but it's a non-intentional mistake.

Run this:

    document.body.style.cssText = "background-position: var(--b)";
    document.body.style.backgroundPositionX = "0px";
    document.body.style.cssText;

Expected: "background-position-x: 0px; background-position-y: ;"
Actual: "background-position: var(--b); background-position-x: 0px;"

Another similar case, but here background-position-x is manually inserted at the end: https://searchfox.org/wubkat/rev/7a292520f6b12e8d4d9001d1480474b5c83cb0f8/Source/WebCore/css/StyleProperties.cpp#1890
In bug 190753 I added a check to avoid this if already serialized by the all shorthand, but the background and background-position shorthands are not checked.
In fact this acts as a workaround for bug 247734 so it's not that bad, but both should be fixed.

Run this:

    document.body.style.webkitMask = "none 0px 0px";
    document.body.style.cssText;

Expected: "-webkit-mask: none 0px 0px;"
Actual: "mask-image: none; -webkit-mask: none 0px 0px; -webkit-mask-position-x: 0px; -webkit-mask-position-y: 0px;"

This case doesn't use var(), but it's still a similar problem.
The -webkit-mask shorthand is typically avoided, except for -webkit-mask-clip.
So first mask-image avoids -webkit-mask-clip and serializes as-is, then -webkit-mask-clip sees a possible serialization with -webkit-mask and uses it even if mask-image has already present, and then following longhands will not realize that they have been covered by -webkit-mask.

There may be similar cases for the mask shorthand but it's serialization seems quite broken.
Comment 1 Oriol Brufau 2022-11-11 10:29:35 PST
First 2 cases addressed by bug 247734.

Now it's:

    document.body.style.cssText = "background-position: var(--b)";
    document.body.style.backgroundPositionX = "0px";
    document.body.style.cssText;

Expected: "background-position-x: 0px; background-position-y: ;"
Actual: "background-position-y: ; background-position-x: 0px;"
So just a non-canonical order.

And I think the 4th case was missing a 1st line, but oh well:

    document.body.style.cssText = "-webkit-mask: none 0px 0px";
    document.body.style.cssText;

Expected: "-webkit-mask: none 0px 0px;"
Actual: "mask-image: none; -webkit-mask-position-x: 0px; -webkit-mask-position-y: 0px; -webkit-mask: none 0px 0px;"
Comment 2 Oriol Brufau 2022-11-14 12:05:14 PST
Order of background-position-x/y fixed by bug 247879.

So it's just with mask properties now.
Comment 3 Radar WebKit Bug Importer 2022-11-17 09:33:16 PST
<rdar://problem/102471167>