Bug 100826

Not sure I agree. The name is pretty clear in that the pseudo element is unknown at the time of parsing the selector. It could be either a custom pseudo element (starts with "x-", a WebKit-specific custom pseudo element (starts with "-webkit-"), or some totally unknown thing. Ohhh... I think I understand what you're saying. Since we have these three choices and they are knowable at the time of parsing, we might distinguish between them. Right?

Yeah.. Actually it is actually used for shadowPseudoId, etc. We can distinguish some specific types, so we should use some good name instead of UNKNOWN. It's sometimes known.

(In reply to comment #2) > Yeah.. Actually it is actually used for shadowPseudoId, etc. > We can distinguish some specific types, so we should use some good name instead of UNKNOWN. > It's sometimes known. I'm not sure what to do. However, I would like to ask one question. If we use some pseudo type for shadow pseudo ids or custom pseudo, i.e. -webkit or x-, we need unknown ids, e.g. -UNKNOWN-UNKNOWN? I have to check CSS spec. However I'm just interested in your idea about such completely unknown ids.

For completely UNKNOWN ids, we can skip using that CSS rule, right? It's because it won't match anything. When parsing succeeded, we should already know the type of id.

(In reply to comment #4) > For completely UNKNOWN ids, we can skip using that CSS rule, right? It's because it won't match anything. > When parsing succeeded, we should already know the type of id. Yes, that's the win I see. In terms of naming, I see three types of "unknown" pseudo-elements: 1) CustomPseudoElement --> CUSTOM 2) WebKitPseudoElement --> WEBKIT_CUSTOM 3) UnknownPseudoElement --> UNKNOWN The interesting bit is that WebKitCustomElement should only match attributes that were set by the UA. We should _not_ allow any random Bob set "-webkit-slider-thumb" on their div and have it matched. That would be bad.

Created attachment 172267 [details] Patch

Comment on attachment 172267 [details] Patch Attachment 172267 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14719717 New failing tests: css3/selectors3/html/css3-modsel-161.html fast/box-sizing/percentage-height.html css3/selectors3/xhtml/css3-modsel-177b.xml css3/selectors3/html/css3-modsel-177b.html fast/dom/shadow/shadow-pseudo-id.html css3/selectors3/xml/css3-modsel-161.xml css3/selectors3/xml/css3-modsel-177b.xml fast/selectors/177b.html css3/selectors3/xhtml/css3-modsel-161.xml

Comment on attachment 172267 [details] Patch Attachment 172267 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14745151 New failing tests: css3/selectors3/html/css3-modsel-161.html fast/box-sizing/percentage-height.html css3/selectors3/xhtml/css3-modsel-177b.xml css3/selectors3/html/css3-modsel-177b.html css3/selectors3/xml/css3-modsel-161.xml css3/selectors3/xml/css3-modsel-177b.xml fast/selectors/177b.html css3/selectors3/xhtml/css3-modsel-161.xml

Created attachment 172454 [details] Patch

Comment on attachment 172267 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=172267&action=review > Source/WebCore/ChangeLog:8 > + We used isUnknownPseudoElement() for these 3 meanings: 1) an element is custom pseudo element (starting with 'x-'), s/a element is/the element is an/ By the way, what is "custom pseudo element"? Is it a standard-mentioned name? > Source/WebCore/css/CSSParser.cpp:10111 > + if (specifiers->isCustomPseudoElement() || specifiers->isWebKitCustomPseudoElement()) { Looks like this combination is frequently used and better to have a shortcut method. Otherwise this change makes the code more complex instead of less-confusing. > Source/WebCore/css/CSSSelectorList.cpp:204 > + return selector->isUnknownPseudoElement() || selector->isWebKitCustomPseudoElement() || selector->isCustomPseudoElement(); Let's define a CSSSelector method instead of scattering this kind of named logic here and there. > LayoutTests/fast/dom/shadow/shadow-pseudo-id.html:30 > +</script> Can we make this test code more descriptive? It's unclear what this test is trying to exercise, which will make it hard to diagnose future regression.

Comment on attachment 172454 [details] Patch unknown -> invalid?

(In reply to comment #10) > By the way, what is "custom pseudo element"? Is it a standard-mentioned name? I've taken it from ShadowDOM spec. https://dvcs.w3.org/hg/webcomponents/raw-file/tip/spec/shadow/index.html#custom-pseudo-elements

I would like to use 'AuthorCustomPseudoElement' instead of 'CustomPseudoElement', and use 'CustomPseudoElement' as (AuthorCustomPseudoElement or WebKitCustomPseudoElement)... Someone can come up with a better name?

(In reply to comment #13) > I would like to use 'AuthorCustomPseudoElement' instead of 'CustomPseudoElement', > and use 'CustomPseudoElement' as (AuthorCustomPseudoElement or WebKitCustomPseudoElement)... > > Someone can come up with a better name? I don't think prefixing the name with Author makes it less clear. The fact that there are WebKit-specific custom pseudo elements is where we need to dig, not the other way around. UserAgentPseudoElement? Also, I really like the "invalid" name. It makes a lot more sense than "unqueriable". It's un-queriable because it's invalid. Better statement of purpose.

Hmm... But we have :invalid pseudo-class already :-(

(In reply to comment #15) > Hmm... But we have :invalid pseudo-class already :-( That's ok. That's different. Look here: http://dev.w3.org/csswg/selectors4/#invalid

Created attachment 172474 [details] Patch

Another try

Comment on attachment 172474 [details] Patch Looks fine. wait test green being show up.

Comment on attachment 172474 [details] Patch Clearing flags on attachment: 172474 Committed r133577: <http://trac.webkit.org/changeset/133577>

All reviewed patches have been landed. Closing bug.