[webkit-reviews] review denied: [Bug 83438] querySelectorAll unable to find SVG camelCase elements imported into HTML : [Attachment 185355] Patch V6

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has denied Alexandru Chiculita
<achicu at adobe.com>'s request for review:
Bug 83438: querySelectorAll unable to find SVG camelCase elements imported into
HTML
https://bugs.webkit.org/show_bug.cgi?id=83438

Attachment 185355: Patch V6
https://bugs.webkit.org/attachment.cgi?id=185355&action=review

------- Additional Comments from Antti Koivisto <koivisto at iki.fi>
View in context: https://bugs.webkit.org/attachment.cgi?id=185355&action=review


> Source/WebCore/ChangeLog:14
> +	   However, non-HTML elements embedded in HTML need to follow the XML
rule instead. They need to be matched using case-sensitive comparison.
> +	   That's aligned with the behavior of FF and IE10. However, because
the CSS parser already lower-cased the tag name, the matching never
> +	   works for elements like "foreignObject" or "textPath", as their
canonical form has both lower-case and upper-case letters.

It may be aligned with FF and IE10 but is it correct? It is still not clear to
me that these complications make sense. Shouldn't everything within a document
type match the same?

> Source/WebCore/css/CSSGrammar.y.in:1215
> +	       AtomicString mixedCaseTagName;
> +	       if (parser->m_context.isHTMLDocument && !$2.isLowerCase()) {
> +		   mixedCaseTagName = $2;
> +		   $2.lower();
> +	       }
> +	       parser->updateSpecifiersWithElementName($1, $2,
mixedCaseTagName, $$);

So if someone writes selectors in capital letters DIV { foo: bar } he will end
up with rare datas. That seems bad. Can this be limited to known SVG names only
or something?

> Source/WebCore/css/SelectorChecker.cpp:203
> +	   case CSSSelector::MixedCaseTag:
> +	       if (!fastCheckSingleSelector<checkMixedCaseTagValue>(selector,
element, topChildOrSubselector, topChildOrSubselectorMatchElement))
> +		   return false;
> +	       break;

Please don't add this to the fast path. These are supposed to be rare (they are
rare in data after all!) and making fast path bigger can make it less fast.