1401449 - stylo: Convert :-moz-list-bullet and :-moz-list-number to (non-web-exposed) inheriting anonymous boxes and eliminate the lazy pseudo cache

Status: RESOLVED WONTFIX

(Core :: CSS Parsing and Computation, enhancement, P4)

Description

[Bobby Holley (:bholley)]

See bug 1401317 comment 11. There is some webcompat risk here, but it would simplify various things and save memory.

Comment 1

[Bobby Holley (:bholley)]

Doing this on top of bug 1429126.

Comment 2

[Bobby Holley (:bholley)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f6aa61f57fc6bc23f2f0268c9c3be3e60c57e15b

Comment 3

[Bobby Holley (:bholley)]

Attachment: Part 1 - Convert -moz-list-bullet and -moz-list-number to (non-web-exposed) inheriting anonymous boxes. v1

MozReview-Commit-ID: 91X3bG7MAsg

Comment 4

[Bobby Holley (:bholley)]

Attachment: Part 2 - Eliminate lazy pseudo style cache. v1

MozReview-Commit-ID: 8rR8tz6Rq7w

Comment 5

[Bobby Holley (:bholley)]

Attachment: Part 2 - Eliminate lazy pseudo style cache. v2

MozReview-Commit-ID: 8rR8tz6Rq7w

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8941273 [details] [diff] [review]
Part 1 - Convert -moz-list-bullet and -moz-list-number to (non-web-exposed) inheriting anonymous boxes. v1

Review of attachment 8941273 [details] [diff] [review]:
-----------------------------------------------------------------

Code-wise looks ok, though I'm surprised to not see any test-changes. Also, could you send an intent to unship to dev-platform, and wait a bit for replies or concerns to come in in case there are any?

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8941273 [details] [diff] [review]
Part 1 - Convert -moz-list-bullet and -moz-list-number to (non-web-exposed) inheriting anonymous boxes. v1

Review of attachment 8941273 [details] [diff] [review]:
-----------------------------------------------------------------

Also, thinking a bit more about this change, it's probably kinda weird how the restyling setup ends up... Because this is still restyling them from UpdatePseudoElementStyles, and AppendDirectlyOwnedAnonBoxes doesn't append it...

Boris, wdyt of the patch as-is? Maybe we should change them to actually make bullets use the anon box machinery instead...

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8941284 [details] [diff] [review]
Part 2 - Eliminate lazy pseudo style cache. v2

Review of attachment 8941284 [details] [diff] [review]:
-----------------------------------------------------------------

This is fine of course, once the previous patch is addressed.

Maybe worth mentioning in the commit message that the other lazy pseudos don't seem worth sharing.

Comment 9

[Bobby Holley (:bholley)]

NI Jason, just as a heads-up that -moz-list-bullet and -moz-list-number will no longer be content-exposed and thus the fuzzers probably shouldn't bother generating them anymore once this lands.

Comment 10

[Bobby Holley (:bholley)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d5ab7f4221fc074ef69c07be0580e3b42b44c27c

Comment 11

[Jason Kratzer [:jkratzer]]

Noted - thanks.  Eventually, supported selectors will be detected as part of our CI process but for now, I'll go ahead and manually remove them.

Comment 12

[Bobby Holley (:bholley)]

While drafting the intent-to-unship, I realized that the current implementation lines up pretty well with ::marker, which we may want to implement at some point. Emilio, wdyt?

Comment 13

[Bobby Holley (:bholley)]

(and Jason, maybe don't remove them just yet, since there's a chance we might not land this)

Comment 14

[Bobby Holley (:bholley)]

Hm, I suppose now that we have the patches in bug 1429126, the reasoning for two separate linked lists (in bug 1368291 comment 5) no longer holds, and we could just merge the lazy pseudo cache with the anonymous box cache. I'll go ahead and do that.

Comment 15

[Bobby Holley (:bholley)]

(My patches for this bug, which I will probably abandon, are at https://github.com/bholley/gecko/commits/list_bullet_and_number_to_anon_box )

Comment 16

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8941273 [details] [diff] [review]
Part 1 - Convert -moz-list-bullet and -moz-list-number to (non-web-exposed) inheriting anonymous boxes. v1

Worth checking whether people style these in the wild and why?

And yes, ::marker would likely have similar issues in terms of sharing, so those might be worth solving anyway.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Fwiw, a least github searches show very few hits for these pseudo-elements...

Comment 18

[Bobby Holley (:bholley)]

I'm going to try merging the caches in bug 1429529, and leaving moz-list-{bullet,number} alone.

Comment 19

[Emilio Cobos Álvarez (:emilio)]

If we want to implement ::marker I agree this problem needs to be solved regardless. But if we can make this problem not exist by merging the caches, that's even better, I agree :)

Comment 20

[Bobby Holley (:bholley)]

OK, let's take the approach in the other bug then.

So Jason, the final word here is that :-moz-list-bullet and :-moz-list-number should not be removed from the fuzzer, since they'll still be valid (and fruitful for fuzz bugs). NI-ing just to be extra sure you see this. :-)

Comment 21

[Jason Kratzer [:jkratzer]]

Got it - thanks!