Summary: | XPath should support custom namespace resolvers | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Anders Carlsson <andersca> | ||||||||
Component: | DOM | Assignee: | Alexey Proskuryakov <ap> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | cdumez, ian, jwmcpeak, yt.dev | ||||||||
Priority: | P2 | ||||||||||
Version: | 420+ | ||||||||||
Hardware: | Mac | ||||||||||
OS: | OS X 10.4 | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 10489, 12584 | ||||||||||
Attachments: |
|
Description
Anders Carlsson
2006-05-08 14:27:03 PDT
Created attachment 10015 [details]
A quick HTML file showing the use of a user-defined function for the namespace resolver. Works in Gecko and Opera.
The W3C specification states that user-defined functions can be used as namespace resolvers in XPathEvaluator.evaluate() for ECMAScript. This test case shows this usage, and works in Gecko and Opera. When ran in WebKit, a NAMESPACE_ERR error is thrown.
*** Bug 12413 has been marked as a duplicate of this bug. *** Created attachment 12892 [details]
proposed fix
Comment on attachment 12892 [details]
proposed fix
Looks good.
I don't see make file changes for the platforms other than Macintosh.
+String JSCustomXPathNSResolver::lookupNamespaceURI(const String& prefix)
+{
+ String result;
Would be better to declare this closer to where it's used.
+ RefPtr<JSCustomXPathNSResolver> selfProtector(this);
Is there a way to avoid this? Can we just get the values out of member variables and then not access this after calling the function?
Is there a way to cut down on the amount of custom code that's in the CodeGeneratorJS.pm and CodeGeneratorObjC.pm scripts? Can we make helper functions for the JSCustomXPathNSResolver path so that a little more of the work is in a .cpp file rather than inside a script?
The header guards in NativeXPathNSResolver.h say JSNativeXPathNSResolver_h.
I'm going to say review- even though this is very close to a review+.
Created attachment 12910 [details] proposed fix (In reply to comment #4) > I don't see make file changes for the platforms other than Macintosh. Done. > Would be better to declare this closer to where it's used. Moved it a little down. > + RefPtr<JSCustomXPathNSResolver> selfProtector(this); > > Is there a way to avoid this? Can we just get the values out of member > variables and then not access this after calling the function? I don't know for sure - took it from JSAbstractEventListener::handleEvent(). I suppose it may be needed to protect the frame. Would be nice if these functions could share more code some day, but I don't feel myself familiar enough with this code to refactor it. > Is there a way to cut down on the amount of custom code that's in the > CodeGeneratorJS.pm and CodeGeneratorObjC.pm scripts? Can we make helper > functions for the JSCustomXPathNSResolver path so that a little more of the > work is in a .cpp file rather than inside a script? I added a create() function to JSCustomXPathNSResolver, and that saved a few lines in generated code, but not much - so maybe I don't fully understand your suggestion. > The header guards in NativeXPathNSResolver.h say JSNativeXPathNSResolver_h. Fixed. Also added BLOCK_OBJC_EXCEPTIONS to the ObjC resolver. Comment on attachment 12910 [details]
proposed fix
When it comes to reducing the amount of code in CodeGeneratorJS.pm, I'm suggesting that JSCustomXPathNSResolver have a function that can do this entire job:
+ push(@implContent, " RefPtr<XPathNSResolver> customResolver;\n");
+ push(@implContent, " XPathNSResolver* resolver = toXPathNSResolver(args[$paramIndex]);\n");
+ push(@implContent, " if (!resolver) {\n");
+ push(@implContent, " customResolver = JSCustomXPathNSResolver::create(exec, args[$paramIndex]);\n");
+ push(@implContent, " if (exec->hadException())\n");
+ push(@implContent, " return jsUndefined();\n");
+ push(@implContent, " resolver = customResolver.get();\n");
+ push(@implContent, " }\n");
so that CodeGeneratorJS.pm has just a single function call, check for exception/null and return of jsUndefined(), which amounts to about half as much code as what's there now.
Seems like you could do something similar with CodeGeneratorObjC.pm, with a single function that returns a PassRefPtr<XPathNSResolver>.
+#include "XPathNSResolver.h"
+
+#include <wtf/PassRefPtr.h>
+#include <wtf/RefPtr.h>
This requires only Forward.h, not PassRefPtr.h (occurs twice).
r=me
Committed revision 19398. (In reply to comment #6) > so that CodeGeneratorJS.pm has just a single function call, check for > exception/null and return of jsUndefined(), which amounts to about half as much > code as what's there now. I see. I didn't want to make JSCustomXPathNSResolver know about wrapped native resolvers - seems outside its area of responsibility. > This requires only Forward.h, not PassRefPtr.h (occurs twice). Fixed. Mass moving XML DOM bugs to the "DOM" Component. |