[webkit-reviews] review granted: [Bug 30969] A no-prefix XPath node test should not match no-namespace elements in HTML document : [Attachment 42243] proposed patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Alexey Proskuryakov <ap at webkit.org>'s
request for review:
Bug 30969: A no-prefix XPath node test should not match no-namespace elements
in HTML document
https://bugs.webkit.org/show_bug.cgi?id=30969

Attachment 42243: proposed patch
https://bugs.webkit.org/attachment.cgi?id=42243&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> -	       if (node->isHTMLElement() && node->document()->isHTMLDocument())
{
> -		   // Paths without namespaces should match HTML elements in
HTML documents despite those having an XHTML namespace. Names are compared
case-insensitively.
> -		   return
equalIgnoringCase(static_cast<Element*>(node)->localName(), name) &&
(namespaceURI.isNull() || namespaceURI == node->namespaceURI());
> +	       if (node->document()->isHTMLDocument()) {
> +		   if (node->isHTMLElement()) {
> +		       // Paths without namespaces should match HTML elements
in HTML documents despite those having an XHTML namespace. Names are compared
case-insensitively.
> +		       return
equalIgnoringCase(static_cast<Element*>(node)->localName(), name) &&
(namespaceURI.isNull() || namespaceURI == node->namespaceURI());
> +		   } else {
> +		       // An expression without any prefix shouldn't match
no-namespace nodes (because HTML5 says so).
> +		       return static_cast<Element*>(node)->hasLocalName(name)
&& namespaceURI == node->namespaceURI() && !namespaceURI.isNull();
> +		   }

We normally don't do else after return.

It would be nice if the test case covered a few more cases than just the
bug-fixed one.

r=me