Bug 12733

Summary: XPath namespace is attached to a wrong step
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: XMLAssignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
Bug Depends on:    
Bug Blocks: 12584    
Attachments:
Description Flags
proposed fix
mjs: review-
proposed fix mjs: review+

Alexey Proskuryakov
Reported 2007-02-11 01:47:18 PST
When parsing paths such as "//svg:g[@id = 'r_00']", the namespace gets attached to the predicate, incorrectly resulting in something like "//g[@svg:id = 'r_00']".
Attachments
proposed fix (11.20 KB, patch)
2007-02-11 03:06 PST, Alexey Proskuryakov 		mjs: review-
DetailsFormatted DiffDiff
proposed fix (11.58 KB, patch)
2007-02-13 12:45 PST, Alexey Proskuryakov 		mjs: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1 2007-02-11 03:06:10 PST
Created attachment 13116 [details] proposed fix Suggestions are welcome on how to reduce code duplication in XPathGrammar.y. I thought about changing the type of NodeTest to a (string, namespace) pair, but this seemed unfair to other tests that do not use a namespace.
Maciej Stachowiak
Comment 2 2007-02-11 19:18:54 PST
Comment on attachment 13116 [details] proposed fix You could eliminate the duplication by adding an OptionalPredicateList production. r- to consider this, although your fix seems fine as-is.
Alexey Proskuryakov
Comment 3 2007-02-13 12:45:08 PST
Created attachment 13152 [details] proposed fix Thank you - did that!
Darin Adler
Comment 4 2007-02-15 10:03:00 PST
Comment on attachment 13152 [details] proposed fix The only reason I haven't marked this patch reviewed yet is that I'm not yet sure about the storage management in XPathGrammar.y -- I don't understand the rules about when a call to deleteString is needed and when it's not and I'd need to before I could mark this reviewed.
Maciej Stachowiak
Comment 5 2007-02-17 08:20:22 PST
Comment on attachment 13152 [details] proposed fix Darin has a good point. As it is, it looks like the YYABORT calls may leak. On the other hand, it looks to me like none of this is new with Alexey's patch. It might be worth investigating separately. So r=me
Sam Weinig
Comment 6 2007-02-17 14:21:54 PST
Landed in r19684.
Note You need to log in before you can comment on or make changes to this bug.