WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
231340
[CSS Cascade Layers] Update CSSOM to the spec
https://bugs.webkit.org/show_bug.cgi?id=231340
Summary
[CSS Cascade Layers] Update CSSOM to the spec
Simon Fraser (smfr)
Reported
2021-10-06 16:58:01 PDT
Per CSS WG resolution:
https://github.com/w3c/csswg-drafts/issues/6576#issuecomment-937246847
Attachments
Patch
(64.07 KB, patch)
2021-12-08 05:20 PST
,
Antti Koivisto
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Patch
(64.61 KB, patch)
2021-12-08 06:09 PST
,
Antti Koivisto
no flags
Details
Formatted Diff
Diff
Patch
(64.67 KB, patch)
2021-12-08 06:39 PST
,
Antti Koivisto
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2021-10-06 16:58:29 PDT
<
rdar://problem/83958697
>
Antti Koivisto
Comment 2
2021-12-08 05:20:20 PST
Created
attachment 446350
[details]
Patch
Antti Koivisto
Comment 3
2021-12-08 06:09:32 PST
Created
attachment 446353
[details]
Patch
Antti Koivisto
Comment 4
2021-12-08 06:39:42 PST
Created
attachment 446357
[details]
Patch
EWS
Comment 5
2021-12-08 08:56:20 PST
Committed
r286657
(
244970@main
): <
https://commits.webkit.org/244970@main
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 446357
[details]
.
Darin Adler
Comment 6
2021-12-08 09:00:57 PST
Comment on
attachment 446357
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=446357&action=review
> Source/WebCore/inspector/InspectorStyleSheet.cpp:455 > + auto layerName = downcast<CSSImportRule>(parentRule)->layerName(); > + if (!layerName.isNull()) {
Hey Antti, when making a change like this, please consider the modern C++ way to keep this scoped like it was before: if (auto layerName = downcast<CSSImportRule>(parentRule)->layerName(); !layerName.isNull()) { I think it works and makes the code better even though it can be a little hard to spot the code after the semicolon.
Patrick Angle
Comment 7
2021-12-08 09:12:38 PST
Comment on
attachment 446357
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=446357&action=review
> Source/WebCore/inspector/InspectorStyleSheet.cpp:481 > + layerRulePayload->setText(downcast<CSSLayerBlockRule>(parentRule)->name());
The Inspector frontend expects an anonymous layer will not provide the optional `text` parameter. Since this has already landed I've opened
Bug 234012
to track cleaning this up.
Antti Koivisto
Comment 8
2021-12-08 10:49:12 PST
> Hey Antti, when making a change like this, please consider the modern C++ > way to keep this scoped like it was before: > > if (auto layerName = downcast<CSSImportRule>(parentRule)->layerName(); > !layerName.isNull()) { > > I think it works and makes the code better even though it can be a little > hard to spot the code after the semicolon.
I did consider it and decided it reads better as two lines. There is no benefit from limiting the scope here.
Antti Koivisto
Comment 9
2021-12-08 10:54:52 PST
Comment on
attachment 446357
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=446357&action=review
>> Source/WebCore/inspector/InspectorStyleSheet.cpp:481 >> + layerRulePayload->setText(downcast<CSSLayerBlockRule>(parentRule)->name()); > > The Inspector frontend expects an anonymous layer will not provide the optional `text` parameter. Since this has already landed I've opened
Bug 234012
to track cleaning this up.
layerName() returns an empty string for anonymous layers. Maybe best to handle that in the frontend.
Antti Koivisto
Comment 10
2021-12-08 11:00:21 PST
> layerName() returns an empty string for anonymous layers. Maybe best to > handle that in the frontend.
Or to be exact CSSLayerBlockRule::name() returns empty string for anonymous layer (and never a null string). CSSImportRule::layerName() returns null string for no layer specified case and empty string for anonymous layer case. CSSLayerStatementRule::nameList() returns a vector with at least one item and no empty or null strings.
Patrick Angle
Comment 11
2021-12-08 11:13:55 PST
> > layerName() returns an empty string for anonymous layers. Maybe best to > > handle that in the frontend. > > Or to be exact CSSLayerBlockRule::name() returns empty string for anonymous > layer (and never a null string). > > CSSImportRule::layerName() returns null string for no layer specified case > and empty string for anonymous layer case. > > CSSLayerStatementRule::nameList() returns a vector with at least one item > and no empty or null strings.
Makes sense - given this I think I will go ahead and just teach the frontend how to handle an empty string here. Thanks for the clarification!
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug