The default bug view has changed. See this FAQ.

document.querySelectorAll now throws when used with a proxy as argument

RESOLVED FIXED in mozilla15

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ochameau, Assigned: ejpbruel)

Tracking

unspecified
mozilla15
x86_64
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [js:p1:fx16])

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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.
(Reporter)

Updated

5 years ago
Blocks: 703537
Eddy, is this intended behavior?
Whiteboard: [js:investigate][js:p1:fx16]
(Assignee)

Comment 2

5 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.
Whiteboard: [js:investigate][js:p1:fx16] → [js:p1:fx16]
(Assignee)

Comment 3

5 years ago
Created attachment 626131 [details] [diff] [review]
Patch to be reviewed

Looks like I was right. This patch fixes the regression as mentioned in my previous comment.
Attachment #626131 - Flags: review?(bobbyholley+bmo)
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

5 years ago
Pushed http://hg.mozilla.org/integration/mozilla-inbound/rev/981eaf7b3333
https://hg.mozilla.org/mozilla-central/rev/981eaf7b3333
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
You need to log in before you can comment on or make changes to this bug.