Bug 247741

Summary: cssText can serialize duplicated declarations
Product: WebKit Reporter: Oriol Brufau <obrufau>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: NEW    
Severity: Normal CC: webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 247734    
Bug Blocks:    

Oriol Brufau
Reported 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.
Attachments
Oriol Brufau
Comment 1 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;"
Oriol Brufau
Comment 2 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.
Radar WebKit Bug Importer
Comment 3 2022-11-17 09:33:16 PST
Note You need to log in before you can comment on or make changes to this bug.