Bug 236272

Summary:

[css] Can't revert low-priority properties like text-decoration

Product:

WebKit

Reporter:

Oriol Brufau <obrufau>

Component:

CSS

Assignee:

Oriol Brufau <obrufau>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

clopez, darin, ews-watchlist, graouts, koivisto, mmaxfield, simon.fraser, webkit-bug-importer, youennf

Priority:

Keywords:

BrowserCompat, InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://github.com/web-platform-tests/wpt/pull/33036

Bug Depends on:

237175

Bug Blocks:

236199

Attachments:

Description	Flags
testcase	none
Patch	none
Patch	none
Patch	koivisto: review-

Oriol Brufau

Reported 2022-02-07 16:39:42 PST

Created attachment 451172 [details] testcase All of these should look identical: foo foo foo foo foo 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	Details Formatted Diff Diff
Patch (9.05 KB, patch) 2022-03-14 16:35 PDT, Oriol Brufau	no flags	Details Formatted Diff Diff
Patch (10.61 KB, patch) 2022-03-14 17:37 PDT, Oriol Brufau	koivisto: review-	Details Formatted Diff Diff
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.