1265639 - Add support for pseudo element aliases

Status: RESOLVED DUPLICATE of bug 205202

(Core :: CSS Parsing and Computation, defect)

Description

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

To retain the backward compatibility when working on bug 775235 for pseudo elements or pseudo classes, both the prefix version and unprefix version need to be supported. That is, we need an alias mechanism for pseudo element and class.

This will be needed for bug 205202 (::-moz-list-bullet and ::marker) and bug 509958 (::-moz-selection and ::selection)

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Is it complicated enough that a "mechanism" is needed?  In the past, I think we've just had two pseudo-classes in the pseudo-class list and implemented both the same way.

Comment 2

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Take ::-moz-list-bullet for example, when constructing nsBulletFrame, we need to use CSSPseudoElementType::mozListBullet type to get the correct style for the nsBulletFrame. If we then implement ::maker type, we cannot just use the marker pseudo type to construct the nsBulletFrame. If we do, all the css rules for ::-moz-list-bullet will not be applied.

[1] https://dxr.mozilla.org/mozilla-central/rev/67ac40fb8f680ea5e03805552187ba1b5e8392a1/layout/generic/nsBlockFrame.cpp#6911-6928

Comment 3

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Comment #1 is correct that pseudo-classes do not need an alias mechanism, but pseudo-elements are still needed it because frame code often uses CSSPseudoElementType to resolve the correct style context.

Comment 4

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: MozReview Request: Bug 1265639 - Support pseudo element aliases.

The idea is to support parsing both pseudo-element and the alias, but
given both of them the same pseudo-element type via
nsCSSPseudoElements::GetPseudoType().

Review commit: https://reviewboard.mozilla.org/r/51021/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/51021/

Comment 5

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Comment on attachment 8749528 [details]
MozReview Request: Bug 1265639 - Support pseudo element aliases.

The idea behind the patch in comment 4, and un-prefixing  ::-moz-selection (bug 509958) is a possible use case.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

What feedback are you looking for?

(It also seem like whether to support ::selection is a separate question from how an alias mechanism should work, so they should be separate patches.)

Comment 7

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

I'm looking for the feedback of alias mechanism implementation.  If it's on the right track, we might need to add another bool preference argument to the CSS_PSEUDO_ELEMENT_ALIAS like we did nsCSSPropList.h.

Right, the support for ::selection should in a separate patch in bug 509958.  I include it in the patch to demonstrate the alias usage.  However, the mechanism might not worth much right now without a first use case.  (::marker might be another good candidate, but I feel it's more complicated to implement.)

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8749528 [details]
MozReview Request: Bug 1265639 - Support pseudo element aliases.

Sorry for the delay getting to this.

I think the idea of having pseudo-element aliases is fine.  However, I would prefer not to expose them as public names on nsCSSPseudoElements::, since that would tend to lead people to incorrectly use them in code, which would result in their not doing anything (e.g., testing GetPseudo() on a style context against a member of nsCSSPseudoElements).

If necessary, they could be part of their own atom list (nsCSSPseudoElementAliases::) if there's a need to have them atomized, although it's not clear to me there's any need for that.  It might be fine if they're just included more directly into the code that converts aliases to the real values (currently nsCSSPseudoElements::GetPseudoType).

The other problem here is that when an alias is used, I think you're creating a style context that has the unaliased GetPseudoType() but the aliased GetPseudo(), since you're doing the alias handling in nsCSSPseudoElements::GetPseudoType.  In order to follow this approach without risk of code all over the tree getting aliases wrong, we might also need to remove the remaining uses of mPseudoTag from nsStyleContext, and switch all of them (and uses of GetPseudo(), of which there are many) to using GetPseudoType().  It's not clear to me whether this will be trivial or whether it will be hard to do.  In the long term, it is probably what we'd like to do; having both GetPseudo() and GetPseudoType() should be unnecessary.

If it's hard, it might make more sense to do the alias handling earlier in the parsing process.  (In that case, we'd also need to make sure that aliases are handled correctly for getComputedStyle.)

Comment 9

[Mats Palmgren (inactive)]

Is this bug still relevant?

FYI, ::-moz-list-* are now aliased to ::marker in the parser:
https://searchfox.org/mozilla-central/rev/9ee63566281365f26e7a4b06c9d4e2219e64c3e8/servo/components/style/gecko/pseudo_element_definition.mako.rs#205-206

Comment 10

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

I don't think this bug is relevant anymore because bug 205202 has fulfilled the needs for pseudo element aliases. And my patch is against the old (removed) style system.

Comment 11

[ExE Boss]

In that case, this should probably be marked as a duplicate of bug 205202.