Closed
Bug 616992
Opened 14 years ago
Closed 14 years ago
DOM classes have no properties when accessed from a sandbox
Categories
(Core :: XPConnect, defect)
Core
XPConnect
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | betaN+ |
People
(Reporter: asaf, Assigned: mrbkap)
References
Details
(Keywords: regression, Whiteboard: [hardblocker])
Attachments
(2 files)
7.21 KB,
patch
|
peterv
:
review+
jst
:
review+
|
Details | Diff | Splinter Review |
2.55 KB,
patch
|
mrbkap
:
review+
gal
:
review+
|
Details | Diff | Splinter Review |
If a the prototype of a sandbox is set to a dom window, the window's "dom classes" properties (eg. window.NodeFilter) are accessible within the sandbox context. However, the properties of these classes are inaccessible.
To reproduce, try this code in the js console:
alert('keys: ' + Object.keys(NodeFilter));
var sandbox = Components.utils.Sandbox(window, { sandboxPrototype: window });
var codeStr = "alert('keys in sandbox: ' + Object.keys(NodeFilter));";
Components.utils.evalInSandbox(codeStr, sandbox);
Updated•14 years ago
|
blocking2.0: --- → ?
Assignee | ||
Comment 1•14 years ago
|
||
When an object passes across a compartment boundary (such as between the chrome compartment and the sandbox) we attempt to rewrap it. Constructors didn't have a precreate hook, so we'd recreate the constructor in the sandbox's compartment, but the newly created object didn't have the PostCreatePrototype-created properties. This patch gives constructors a PreCreate hook, so we end up creating a transparent wrapper for the sandbox instead of a whole new object.
Comment 2•14 years ago
|
||
This is a compartments regression, need to fix this for Firefox 4.
blocking2.0: ? → betaN+
Keywords: regression
Updated•14 years ago
|
Attachment #495702 -
Flags: review?(Olli.Pettay) → review?(peterv)
Comment 3•14 years ago
|
||
(In reply to comment #1)
> but the newly created object didn't have the
> PostCreatePrototype-created properties.
Trying to understand that, why not?
Updated•14 years ago
|
Summary: DOM classes have no properties when accessed from a sandbox → [needs review] DOM classes have no properties when accessed from a sandbox
Comment 5•14 years ago
|
||
mrbkap, can you answer peterv's question above please?
Assignee | ||
Comment 6•14 years ago
|
||
(In reply to comment #3)
> Trying to understand that, why not?
Because all we're wrapping is the constructor, we never actually create an object (or the real prototype for one). Furthermore, nsDOMConstructorSH::PostCreatePrototype purposely doesn't forward to nsDOMClassInfo::PostCreatePrototype.
Updated•14 years ago
|
Whiteboard: hardblocker
Updated•14 years ago
|
Whiteboard: hardblocker → [hardblocker]
Updated•14 years ago
|
Attachment #495702 -
Flags: review+
Comment 7•14 years ago
|
||
With this additional change we get through all unit tests (didn't run talos) on Linux and Mac, but for some reason this (and the original patch as well) seems to cause an orange in xpcshell tests on windows only (?). The failure is in test_resource_async.js, here's the logs:
With my additional changes:
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1294455964.1294458187.853.gz&fulltext=1
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1294458189.1294459682.9181.gz&fulltext=1
W/o my additional changes:
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1294434465.1294437012.26657.gz&fulltext=1
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1294437831.1294439301.5596.gz&fulltext=1
Comment 8•14 years ago
|
||
I tried to reproduce the windows problem locally w/o luck, running all of xpcshell tests succeeds with the two patches in this bug applied, as does running the test that failed on try by itself. Therefore I pushed the two patches to try again and got no failures this time! My best guess is that my previous push was based on a bad changeset, or something.
Either way, I think this is ready to land now, once someone reviews my additional changes.
Updated•14 years ago
|
Attachment #502204 -
Flags: review?(mrbkap)
Comment 9•14 years ago
|
||
Comment on attachment 502204 [details] [diff] [review]
Additional change to make the above get through mochitest.
Looks good to me.
Attachment #502204 -
Flags: review+
Assignee | ||
Updated•14 years ago
|
Attachment #502204 -
Flags: review?(mrbkap) → review+
Comment 10•14 years ago
|
||
Landed on m-c:
http://hg.mozilla.org/mozilla-central/rev/14752eafce65
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•14 years ago
|
Summary: [needs review] DOM classes have no properties when accessed from a sandbox → DOM classes have no properties when accessed from a sandbox
Updated•14 years ago
|
Attachment #495702 -
Flags: review?(peterv) → review+
You need to log in
before you can comment on or make changes to this bug.
Description
•