Summary: | XPath namespace is attached to a wrong step | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexey Proskuryakov <ap> | ||||||
Component: | XML | Assignee: | 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
Alexey Proskuryakov
2007-02-11 01:47:18 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.
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.
Created attachment 13152 [details]
proposed fix
Thank you - did that!
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.
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
|