Add support for pseudo element aliases

RESOLVED DUPLICATE of bug 205202

Status

()

defect
RESOLVED DUPLICATE of bug 205202
3 years ago
13 days ago

People

(Reporter: TYLin, Unassigned)

Tracking

(Blocks 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48 affected)

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
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)
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.
(Reporter)

Comment 2

3 years ago
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
(Reporter)

Comment 3

3 years ago
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.
Summary: Support for pseudo element and pseudo class aliasing → Add support for pseudo element aliases
(Reporter)

Comment 4

3 years ago
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/
(Reporter)

Comment 5

3 years ago
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.
Attachment #8749528 - Flags: feedback?(dbaron)
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.)
Flags: needinfo?(tlin)
(Reporter)

Comment 7

3 years ago
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.)
Flags: needinfo?(tlin) → needinfo?(dbaron)
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.)
Flags: needinfo?(dbaron)
Attachment #8749528 - Flags: feedback?(dbaron) → feedback-

Updated

a year ago
Blocks: 1455565
Flags: needinfo?(aethanyc)
(Reporter)

Comment 10

13 days ago

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.

Status: NEW → RESOLVED
Last Resolved: 13 days ago
Flags: needinfo?(aethanyc)
Resolution: --- → WONTFIX

Comment 11

13 days ago

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

Resolution: WONTFIX → DUPLICATE
Duplicate of bug: 205202
You need to log in before you can comment on or make changes to this bug.