Closed
Bug 757063
Opened 12 years ago
Closed 12 years ago
document.querySelectorAll now throws when used with a proxy as argument
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: ochameau, Assigned: ejpbruel)
References
Details
(Whiteboard: [js:p1:fx16])
Attachments
(1 file)
1.96 KB,
patch
|
bholley
:
review+
|
Details | Diff | Splinter Review |
The following code was used to work in jetpack codebase before bug 703537 landing: var str = "foo"; var proxy = Proxy.create({ get:function (r,n) { // We can't return `str[n]` // Otherwise, it will throw following exception: // "String.prototype.toString called on incompatible Proxy" return str[n].bind(str); } }); document.querySelectorAll(proxy); It now throws following exception: An invalid or illegal string was specified In order to make jetpack content scripts work again with any possible advanced JS usages (like RightJS framework), we will need a way to make querySelectorAll work again with proxies as argument. One another alternative would be to work on bug 738244 and finally get rid of all these JS proxy code, but it would mean breaking old addons not being repacked.
Comment 1•12 years ago
|
||
Eddy, is this intended behavior?
Updated•12 years ago
|
Whiteboard: [js:investigate][js:p1:fx16]
Assignee | ||
Comment 2•12 years ago
|
||
This is probably caused by my latest patch for direct proxies. In short, ScriptedProxyHandler now inherits from IndirectProxyHandler, instead of BaseProxyHandler (the new name for ProxyHandler). Unlike ProxyHandler, IndirectProxyHandler assumes that the notion of a wrapped object, or target, exists. This allows IndirectProxyHandler to forward the Spidermonkey extension traps to the target, whereas ProxyHandler would just throw. In other words, the behavior of the SpiderMonkey extension traps for ScriptedProxyHandler changed with this patch from throwing to forwarding. In addition, ScriptedProxyHandler doesn't yet have the notion of a target, so currently the extension traps will be forwarded to the handler instead. This is definitely not the correct behavior (for instance, calling toString on a function proxy will currently yield [Object]), but it didn't seem to cause any regressions on try. Moreover, the problem should resolve itself as soon as the ScriptedProxyHandler is updated to have a separate target and handler, which will happen in a future patch. This patch depends on the current one, though, so backing that out isn't really an option. If the problem is indeed what I think it is, a quick fix should be to overload the SpiderMonkey extension traps on the ScriptedProxyHandler to explicitly forward to BaseProxyHandler (which was the original behavior). This should allow us to proceed with the work on direct proxies for the time being without breaking any existing code.
Updated•12 years ago
|
Whiteboard: [js:investigate][js:p1:fx16] → [js:p1:fx16]
Assignee | ||
Comment 3•12 years ago
|
||
Looks like I was right. This patch fixes the regression as mentioned in my previous comment.
Attachment #626131 -
Flags: review?(bobbyholley+bmo)
Comment 4•12 years ago
|
||
Comment on attachment 626131 [details] [diff] [review] Patch to be reviewed http://zipmeme.com/uploads/generated/g1335642612204169554.jpg
Attachment #626131 -
Flags: review?(bobbyholley+bmo) → review+
Assignee | ||
Comment 5•12 years ago
|
||
Pushed http://hg.mozilla.org/integration/mozilla-inbound/rev/981eaf7b3333
Comment 6•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/981eaf7b3333
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
You need to log in
before you can comment on or make changes to this bug.
Description
•