Bug 12452 - XPath id() function is not supported
Summary: XPath id() function is not supported
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: XML (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on:
Blocks: 12450
  Show dependency treegraph
 
Reported: 2007-01-28 12:32 PST by Alexey Proskuryakov
Modified: 2007-01-30 11:21 PST (History)
0 users

See Also:


Attachments
implement id() (7.08 KB, patch)
2007-01-30 10:48 PST, Alexey Proskuryakov
darin: review+
Details | Formatted Diff | Diff
proposed fix (7.26 KB, patch)
2007-01-30 11:13 PST, Alexey Proskuryakov
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2007-01-28 12:32:59 PST
See <http://www.w3.org/TR/xpath#function-id>. This causes a failure in "dom/svg/level3/xpath/Conformance_ID.svg".
Comment 1 Alexey Proskuryakov 2007-01-30 10:48:37 PST
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.
Comment 2 Alexey Proskuryakov 2007-01-30 10:51:58 PST
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.
Comment 3 Darin Adler 2007-01-30 10:52:56 PST
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.
Comment 4 Alexey Proskuryakov 2007-01-30 11:13:45 PST
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.
Comment 5 Darin Adler 2007-01-30 11:17:31 PST
Comment on attachment 12801 [details]
proposed fix

This one looks good too!

r=me
Comment 6 Alexey Proskuryakov 2007-01-30 11:21:42 PST
Committed revision 19263.