Bug 240356 - [cssom] Computed styles shouldn't index shorthands with 1 longhand
Summary: [cssom] Computed styles shouldn't index shorthands with 1 longhand
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Oriol Brufau
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-05-12 13:13 PDT by Oriol Brufau
Modified: 2022-05-25 08:48 PDT (History)
9 users (show)

See Also:


Attachments
Patch (28.55 KB, patch)
2022-05-23 08:49 PDT, Oriol Brufau
no flags Details | Formatted Diff | Diff
Patch (28.60 KB, patch)
2022-05-25 05:37 PDT, Oriol Brufau
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oriol Brufau 2022-05-12 13:13:39 PDT
A computed style shouldn't index shorthands:

   var props = new Set(getComputedStyle(document.documentElement));
   props.has("margin-left"); // true
   props.has("margin"); // false

However, shorthands that have a single longhand are indexed: https://webkit-search.igalia.com/webkit/rev/0d5437dbaa7bdb52f60b64bc7dd5a39adee14e81/Source/WebCore/css/makeprop.pl#215

   var props = new Set(getComputedStyle(document.documentElement));
   props.has("-webkit-text-orientation"); // true

This is wrong. Being a shorthand of a single property typically happens with legacy shorthands (https://www.w3.org/TR/css-cascade-5/#legacy-shorthand)
In this case, -webkit-text-orientation is the legacy name of text-orientation, but it can't be an alias because they have different syntax.
But aliases are not indexed, so it doesn't make sense that if an alias is turned into a shorthand, it becomes indexed in the computed style.

The list of affected properties can be obtained with

   var el = document.documentElement;
   [...getComputedStyle(el)].filter(prop => {
     el.style.cssText = prop + ": initial";
     return el.style.length === 1 && el.style[0] !== prop;
   });

* page-break-after (legacy shorthand of break-after)
* page-break-before (legacy shorthand of break-before)
* page-break-inside (legacy shorthand of break-inside)
* text-decoration (shorthand of text-decoration-line, it should have more longhands, see bug 230083)
* text-decoration-skip (shorthand of text-decoration-skip-ink, no other longhand has been implemented)
* -webkit-column-break-after (legacy shorthand of break-after)
* -webkit-column-break-before (legacy shorthand of break-before)
* -webkit-column-break-inside (legacy shorthand of break-inside)
* -webkit-text-orientation (legacy shorthand of text-orientation)

Bug 104805 is also turning -webkit-perspective into a legacy shorthand of perspective
Comment 1 Radar WebKit Bug Importer 2022-05-19 13:14:13 PDT
<rdar://problem/93598437>
Comment 2 Oriol Brufau 2022-05-23 08:49:15 PDT
Created attachment 459681 [details]
Patch
Comment 3 Tim Nguyen (:ntim) 2022-05-24 23:29:33 PDT
Comment on attachment 459681 [details]
Patch

r=me
Comment 4 EWS 2022-05-25 05:29:00 PDT
No reviewer information in commit message, blocking PR #None
Comment 5 Oriol Brufau 2022-05-25 05:37:03 PDT
Created attachment 459754 [details]
Patch
Comment 6 EWS 2022-05-25 08:18:16 PDT
Committed r294799 (250955@main): <https://commits.webkit.org/250955@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 459754 [details].