Bug 246239

Summary: ComputedStyleExtractor::copyProperties() shouldn't copy shorthands
Product: WebKit Reporter: Oriol Brufau <obrufau>
Component: CSSAssignee: Oriol Brufau <obrufau>
Status: RESOLVED FIXED    
Severity: Normal CC: webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   

Oriol Brufau
Reported 2022-10-07 16:14:55 PDT
https://searchfox.org/wubkat/source/Source/WebCore/css/ComputedStyleExtractor.cpp#4329-4340 Ref<MutableStyleProperties> ComputedStyleExtractor::copyProperties() { Vector<CSSProperty> list; list.reserveInitialCapacity(numCSSProperties); for (unsigned i = firstCSSProperty; i < lastCSSProperty; ++i) { auto propertyID = convertToCSSPropertyID(i); if (auto value = propertyValue(propertyID)) list.append(CSSProperty(propertyID, WTFMove(value))); } list.shrinkToFit(); return MutableStyleProperties::create(WTFMove(list)); } The loop copies both CSS longhands and shorthands. And the shorthands are not expanded. This is wrong, e.g. let's say that the returned list contains: - margin-top: 1px - margin-right: 2px - margin-bottom: 3px - margin-left: 4px - margin: 1px 2px 3px 4px Then the caller uses setProperty(CSSPropertyMarginTop, "5px"), the list will contain: - margin-top: 5px - margin-right: 2px - margin-bottom: 3px - margin-left: 4px - margin: 1px 2px 3px 4px And then getPropertyValue(CSSPropertyMargin) will be the old "1px 2px 3px 4px" instead of "5px 2px 3px 4px". Also, asText() will produce a string containing both "margin: 1px 2px 3px 4px" twice: one time as a wrong representation of the longhands, and another due to the unexpanded shorthand. Not sure if these problems are actually webexposed since copyProperties() is basically used internally for editing. This seems a regression from bug 198680, since previously in only copied computable properties, which are longhands.
Attachments
Oriol Brufau
Comment 1 2022-10-07 16:26:59 PDT
EWS
Comment 2 2022-10-08 17:32:23 PDT
Committed 255318@main (a97990a34197): <https://commits.webkit.org/255318@main> Reviewed commits have been landed. Closing PR #5157 and removing active labels.
Radar WebKit Bug Importer
Comment 3 2022-10-08 17:33:20 PDT
Note You need to log in before you can comment on or make changes to this bug.