Bug 12452 – XPath id() function is not supported

• Component: XML
• Status: RESOLVED FIXED

[Alexey Proskuryakov]

See <http://www.w3.org/TR/xpath#function-id>. This causes a failure in "dom/svg/level3/xpath/Conformance_ID.svg".

[Alexey Proskuryakov]

Created attachment 12799 [details]
implement id()

I have certain doubts about XPathResult ordering. The existing code expects that it is each function's duty to preserve ordering, but that would be fairly inefficient when the client doesn't need ordered results. Maybe it would be better to re-order results in XPathResult::convertTo().

Similar issues arise for ANY_UNORDERED_NODE_TYPE, FIRST_ORDERED_NODE_TYPE and iterator types - building a full snapshot just to provide one of these is inefficient.

[Alexey Proskuryakov]

This patch doesn't implement one bit of id()-related functionality - this is the only function that returns a node-set, and the existing code doesn't expect that, thus paths including id() don't work. Going to file a follow-up bug.

[Darin Adler]

Comment on attachment 12799 [details]
implement id()

r=me

I'm irritated to see yet-another isWhitespace function. I'd like to see us figure out which flavors of this we need and share central copies with easy-to-understand names.

String is extremely inefficient to append to; it reallocates each time. So a loop that appends the string value and then a space is O(n^2) the size of the string. The way to avoid this is to use a Vector<UChar> instead, and convert to a String at the end with String::adopt.

[Alexey Proskuryakov]

Created attachment 12801 [details]
proposed fix

> I'm irritated to see yet-another isWhitespace function.

  There was a single identical one I could find, in SVGParserUtilities.h. I believe parts of this file should be abstracted out, because they have nothing to do with SVG.

> String is extremely inefficient to append to; it reallocates each time.

  I allowed myself this sloppiness, because I couldn't see how this would noticeably slow down XPath -but OK.

[Darin Adler]

Comment on attachment 12801 [details]
proposed fix

This one looks good too!

r=me

[Alexey Proskuryakov]

Committed revision 19263.