[webkit-reviews] review granted: [Bug 26005] Optimization for XPath //* does not preserve context size : [Attachment 30733] proposed fix

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Alexey Proskuryakov <ap at webkit.org>'s
request for review:
Bug 26005: Optimization for XPath //* does not preserve context size
https://bugs.webkit.org/show_bug.cgi?id=26005

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

------- Additional Comments from Darin Adler <darin at apple.com>
> + * Copyright (C) 2005 Frerich Raabe <raabe at kde.org>
> + * Copyright (C) 2006, 2009 Apple, Inc.

No comma in Apple Inc.

> +    switch (axis) {
> +	   case Step::AttributeAxis:
> +	       return Node::ATTRIBUTE_NODE;
> +	   case Step::NamespaceAxis:
> +	       return Node::XPATH_NAMESPACE_NODE;
> +	   default:
> +	       return Node::ELEMENT_NODE;
> +    }

I like to leave out the "default" case so that gcc can warn me about unhandled
enum values. This is practical when the cases end with return statements, since
the default return statement can go outside the switch, perhaps with an
ASSERT_NOT_REACHED().

> +		   // In XPath land, namespace nodes are not accessible on the
attribute axis.
> +		   if (node->namespaceURI() == "http://www.w3.org/2000/xmlns/")

> +		       return false;

Since namespaces are atomic strings, comparing to an AtomicString constant may
be slightly more efficient, and it would also be nice to have that string in
some common place.

r=me