Reorder ::after and ::picker-icon pseudo-elements [chromium/src : main]

0 views

Skip to first unread message

Joey Arhar (Gerrit)

unread,

1:02 PM (1 hour ago) 1:02 PM

to David Baron, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, dbaro...@chromium.org, dgroga...@chromium.org, blink-...@chromium.org, blink-re...@chromium.org, blink-revie...@chromium.org

Attention needed from David Baron

Joey Arhar added 1 comment

File third_party/blink/renderer/core/dom/node.h

File-level comment, Patchset 4 (Latest):

Joey Arhar . unresolved

So it seems that reordering these pseudos here doesn't actually work as expected.

I looked for other places in the code which might indicate ordering of pseudo-elements, and they all already look like they put ::picker-icon after ::after:

https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/dom/node.cc;l=2832-2842;drc=3c91996c32fb9a4c8d8b76aaa3b6def431dfa80f
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/dom/element.cc;l=5253-5257;drc=c8ca97247e951233ffe4bc32fd223454e9b55b20
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/dom/element.cc;l=5944-5946;drc=c8ca97247e951233ffe4bc32fd223454e9b55b20

Do you have any ideas why this doesn't work? I also verified that the new tests also fail without making any c++ changes.

Open in Gerrit

Related details

Attention is currently required from:

David Baron

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

David Baron (Gerrit)

unread,

1:11 PM (1 hour ago) 1:11 PM

Attention needed from Joey Arhar

David Baron added 1 comment

File third_party/blink/renderer/core/dom/node.h

File-level comment, Patchset 4 (Latest):

Joey Arhar . unresolved

So it seems that reordering these pseudos here doesn't actually work as expected.
I looked for other places in the code which might indicate ordering of pseudo-elements, and they all already look like they put ::picker-icon after ::after:
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/dom/node.cc;l=2832-2842;drc=3c91996c32fb9a4c8d8b76aaa3b6def431dfa80f
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/dom/element.cc;l=5253-5257;drc=c8ca97247e951233ffe4bc32fd223454e9b55b20
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/dom/element.cc;l=5944-5946;drc=c8ca97247e951233ffe4bc32fd223454e9b55b20
Do you have any ideas why this doesn't work? I also verified that the new tests also fail without making any c++ changes.

David Baron

I don't think this covers all of the places that need changing -- in particular I think you still need to modify the stuff in `element.h` and `element.cc` that was changed in https://crrev.com/c/7793851

(I was thinking I might at some point try to hook those up to the same setup but I didn't look into it yet.)