| Summary: | [cssom] Iterating computed style should not include 'all' shorthand | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Oriol Brufau <obrufau> | ||||||||
| Component: | CSS | Assignee: | Oriol Brufau <obrufau> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | darin, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, koivisto, macpherson, menard, webkit-bug-importer | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
Created attachment 458588 [details]
Patch
Comment on attachment 458588 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=458588&action=review > Source/WebCore/css/makeprop.pl:211 > + if ($name eq "all") { > + return 1; > + } Two small thoughts: 1) Is there any way to do this with a property rather than literally hardwiring "all"? 2) Since this is perl, there is a syntax like: return 1 if $name eq "all"; (In reply to Darin Adler from comment #3) > Comment on attachment 458588 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=458588&action=review > > > Source/WebCore/css/makeprop.pl:211 > > + if ($name eq "all") { > > + return 1; > > + } > > Is there any way to do this with a property rather than literally > hardwiring "all"? Do you mean in CSSProperties.json? I guess I could add some flag, Blink uses "computable". But "all" is special anyways, since it has "codegen-properties": { "longhands": [ "all" ] }, which is already hardwired: if ($_ eq "all") { foreach my $propname (@names) { next if (exists $propertiesWithStyleBuilderOptions{$propname}{"longhands"}); next if ($propname eq "direction" || $propname eq "unicode-bidi"); die "Unknown CSS property used in all shorthand: $propname" if !exists($nameToId{$propname}); push(@{$longhandToShorthands{$propname}}, $name); print SHORTHANDS_CPP " CSSProperty" . $nameToId{$propname} . ",\n"; } } Created attachment 458702 [details]
Patch
(In reply to Darin Adler from comment #3) > Comment on attachment 458588 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=458588&action=review > > 1) Is there any way to do this with a property rather than literally > hardwiring "all"? Adding some way to do that can be discussed in bug 239670. > 2) Since this is perl, there is a syntax like: > > return 1 if $name eq "all"; Done, and added comment. Created attachment 458704 [details]
Patch
Committed r293689 (250187@main): <https://commits.webkit.org/250187@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 458704 [details]. |
A computed style should only list declarations for longhand properties, e.g. var cs = getComputedStyle(document.body); var longhands = new Set(cs); longhands.has("margin"); // false longhands.has("margin-left"); // true However, the 'all' shorthand is included: longhands.has("all"); // true, should be false Same with item(): cs.item(4); // "alignment-baseline" cs.item(5); // "all" cs.item(6); // "alt" See https://drafts.csswg.org/cssom/#dom-window-getcomputedstyle > set decls to a list of all longhand properties that are supported CSS > properties, in lexicographical order, with the value being the resolved > value computed for obj using the style rules associated with doc. > Additionally, append to decls all the custom properties whose computed > value for obj is not the guaranteed-invalid value. getComputedStyle().all should continue working of course, but it shouldn't be indexed.