384003 - XPathEvaluator does not accept a Node as NSResolver

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Sjoerd Visscher]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.4) Gecko/20070515 Firefox/2.0.0.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.4) Gecko/20070515 Firefox/2.0.0.4

The XpathEvaluator accepts besides objects created with createNSResolver also any object with a lookupNamespaceURI method.

As any Node has a lookupNamespaceURI method, I expected XPathEvaluator also to accept a Node as a NSResolver, but I get this error:

Exception... "Could not convert JavaScript argument"  nsresult: "0x80570009 (NS_ERROR_XPC_BAD_CONVERT_JS)"

Reproducible: Always

Comment 1

[Alex Vincent [:WeirdAl]]

Sjoerd, can you provide an actual testcase - the XML and JS code you used to generate this error?

Comment 2

[Sjoerd Visscher]

That would be the Xopus product, 1.5Mb of JS code. I don't think that would help you much. Do you have a basic XSLT test case somewhere that I can adapt?

Comment 3

[nanto_vi (TOYAMA Nao)]

Attachment: Simple testcase

This works in Safari 3.1.2 and Opera 9.52.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Per spec what we do is enough, but should be easy to add support for
node as resolver.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Attachment: possible fix

We could do it this way. No need to implement anything new, just make
dom nodes to implement resolver interface.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

The applies still to trunk (with some fuzz).

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

> The XpathEvaluator accepts besides objects created with createNSResolver also
> any object with a lookupNamespaceURI method.

There is no spec that says that this is the case.

It was however recently suggested that nodes should implement XPathNSResolver which would make passing nodes as resolvers work.

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 345716 [details] [diff] [review]
possible fix

Peterv, what do you think?

Comment 9

[Peter Van der Beken [:peterv]]

Comment on attachment 345716 [details] [diff] [review]
possible fix

This is slightly different from what Webkit does, Webkit allows any object that has a lookupNamespaceURI method. In practice it will work the same for now, because our nodes seem to be the only objects that have a lookupNamespaceURI method.

I think we need to make createNSResolver do a QI on the node instead of returning a new nsXPathNSResolver. If we add nsIDOMXPathNSResolver to nsNode3Tearoff then we should remove nsXPathNSResolver (the alternative would be to make nsXPathNSResolver a tearoff of nodes instead). Might also want to add nsIDOMXPathNSResolver to the DOMCI data for all nodes and remove the XPathNSResolver DOMCI data.

Sicking: don't we need to add CanCallerAccess to places where we take a nsIDOMXPathNSResolver if someone passes in a node?

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Actually, XOWs should remove the need for CanCallerAccess, everywhere. Just think we've been conservative elsewhere.

And technically making createNSResolver return a Node would be violating specs. At least the spirit if them.

Comment 11

[Peter Van der Beken [:peterv]]

Comment on attachment 345716 [details] [diff] [review]
possible fix

Hmm, guess so.

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Attachment: up-to-date

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

http://hg.mozilla.org/mozilla-central/rev/48466ec4daa7

Comment 14

[Mike Beltzner [:beltzner, not reading bugmail]]

(This seems to have improved TDHTML on Vista, at least! Good job!)