RESOLVED FIXED 236272
[css] Can't revert low-priority properties like text-decoration 
https://bugs.webkit.org/show_bug.cgi?id=236272
Summary [css] Can't revert low-priority properties like text-decoration
Oriol Brufau
Reported 2022-02-07 16:39:42 PST
Created attachment 451172 [details] testcase All of these should look identical: <u style="all: revert">foo</u> <u style="text-decoration: revert">foo</u> <u style="all: revert-layer">foo</u> <u style="text-decoration: revert-layer">foo</u> <u>foo</u> But only the last one has the underline set in UA origin. That's because text-decoration is low-priority / deferred / applied in parse order. `revert` and `revert-layer` don't seem to work on these properties: https://webkit-search.igalia.com/webkit/rev/30ea950a1b2947097300016afbe68f78665b0e43/Source/WebCore/style/PropertyCascade.cpp#41-73
Attachments
testcase (621 bytes, text/html)
2022-02-07 16:39 PST, Oriol Brufau 		no flags
Details
Patch (10.02 KB, patch)
2022-03-02 15:38 PST, Oriol Brufau 		no flags
DetailsFormatted DiffDiff
Patch (9.05 KB, patch)
2022-03-14 16:35 PDT, Oriol Brufau 		no flags
DetailsFormatted DiffDiff
Patch (10.61 KB, patch)
2022-03-14 17:37 PDT, Oriol Brufau 		koivisto: review-
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2022-02-14 16:40:17 PST
<rdar://problem/88935655>
Oriol Brufau
Comment 2 2022-03-02 15:38:54 PST
Created attachment 453668 [details] Patch
EWS Watchlist
Comment 3 2022-03-02 15:40:43 PST
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Oriol Brufau
Comment 4 2022-03-14 16:35:18 PDT
Created attachment 454635 [details] Patch
Oriol Brufau
Comment 5 2022-03-14 16:41:55 PDT
Just changed the test to use /html/resources/common.js instead of hardcoding a list of elements.
Oriol Brufau
Comment 6 2022-03-14 17:37:04 PDT
Created attachment 454640 [details] Patch
Oriol Brufau
Comment 7 2022-03-14 17:38:25 PDT
Comment on attachment 454640 [details] Patch I forgot that all-prop-revert-layer-noop.html passes after this.
EWS
Comment 8 2022-03-14 18:49:24 PDT
Committed r291260 (248412@main): <https://commits.webkit.org/248412@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 454640 [details].
Antti Koivisto
Comment 9 2022-03-23 06:37:35 PDT
Comment on attachment 454640 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=454640&action=review > Source/WebCore/style/PropertyCascade.cpp:176 > + m_deferredPropertiesIndices.set(id, m_deferredProperties.size()); It is not ok to add a HashMap (and a heap allocation) to this generic code path when it is only needed and used in the rare rollback case. The code should be structured differently.
Antti Koivisto
Comment 10 2022-03-23 09:51:05 PDT
In https://bugs.webkit.org/show_bug.cgi?id=238260 this HashMap is used in sensible way to eliminate the m_deferredProperties member. In that context this patch makes more sense. Still, it is not clear it should be a HashMap.
Note You need to log in before you can comment on or make changes to this bug.