Closed
Bug 760071
Opened 12 years ago
Closed 12 years ago
Debug scope proxies don't let eval-in-frame code define new properties
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: Honza, Assigned: luke)
Details
Attachments
(1 file)
2.61 KB,
patch
|
jimb
:
review+
|
Details | Diff | Splinter Review |
Firebug's test doesn't work since the "TypeError: can't redefine non-configurable property" error appears when evaluating the following script in a frame: if (!window._firebug)window._firebug={}; window._firebug.rerunThis = this; window._firebug.rerunArgs = []; if (arguments && arguments.length) for (var i = 0; i < arguments.length; i++) window._firebug.rerunArgs.push(arguments[i]); window._firebug.rerunFunctionName = onclick; window._firebug.rerunFunction = function _firebugRerun() { onclick.apply(window._firebug.rerunThis, window._firebug.rerunArgs); } See the source here: https://github.com/firebug/firebug/blob/master/extension/content/firebug/js/debugger.js#L445 STR: 1) Install Firebug: http://getfirebug.com/releases/firebug/1.10/firebug-1.10.0a9.xpi 2) Load: http://getfirebug.com/tests/head/script/callstack/3645/issue3645.html 3) Follow the steps on the page -> after clicking re-run, the debugger doesn't halt again -> BUG You can also see the error by installing FBTrace: http://getfirebug.com/releases/fbtrace/1.10/fbTrace-1.10b1.xpi Open FBTrace by "Firebug (icon) menu -> Open Firebug Tracing". In the console check UI_LOOP option (within the Options tab) and watch the console when pressing the rerun button during the test case. The log comes from this source location: https://github.com/firebug/firebug/blob/master/extension/content/firebug/js/debugger.js#L468 The log: debugger.rerun false and result: [object Object] for http://legoas/src/github.com/firebug/firebug/tests/content/script/callstack/3645/issue3645.html - Expand the log - Select the Object tab - Result -> Value -> stringValue == "TypeError: can't redefine non-configurable property 'i'" It works in Firefox 13 Is there something wrong with the evaluated script? Honza
Assignee | ||
Comment 1•12 years ago
|
||
That error pops out when eval-in-frame code tries to redefine a binding in the enclosing scope. Right now, the error is overly conservative (it throws on *any* call to defineProperty on the scope object) since it wasn't even clear how this path would be exercised (normal reads/writes go through get/set). I'll look at what your test-case is doing and probably we can relax the restriction to allow defineProperty when, e.g., only the value changes.
Assignee | ||
Updated•12 years ago
|
Summary: TypeError: can't redefine non-configurable property → Debug scope proxies don't let eval-in-frame code define new properties
Assignee | ||
Comment 2•12 years ago
|
||
Simple enough fix; thanks for the great STR! With this patch, do you see any other problems Jan? The eif-call-newvar change is just reverting the change made in bug 690135.
Comment 3•12 years ago
|
||
Comment on attachment 628845 [details] [diff] [review] fix and tests Review of attachment 628845 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me. Would it be worthwhile to test that a *re*declaration of a variable also works?
Attachment #628845 -
Flags: review?(jimb) → review+
Assignee | ||
Comment 4•12 years ago
|
||
(In reply to Jim Blandy :jimb from comment #3) Heh, well, that actually doesn't work with the patch (that's what the if(has) Throw is guarding against). To make it work, we'd need to do a lot more thinking to make sure the PropertyDescriptors match and then the soon-to-be-landed patches in bug 659577 would need to do the right thing for unaliased variables. All in all, I'd rather not do it if it isn't necessary.
Assignee | ||
Comment 5•12 years ago
|
||
As jimb pointed out on irc, re-defining a var does not call defineProperty, but just sets the existing var, so it should work just fine. Pushed with the suggested redefinition test: https://hg.mozilla.org/integration/mozilla-inbound/rev/6b99d0e6e281
Target Milestone: --- → mozilla15
Comment 6•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6b99d0e6e281
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 7•12 years ago
|
||
(In reply to Luke Wagner [:luke] from comment #2) > Simple enough fix; thanks for the great STR! With this patch, do you see > any other problems Jan? It looks like this will be in Nightly soon so, I'll let you know. Thanks for the quick fix! Honza
Reporter | ||
Comment 8•12 years ago
|
||
Retested with today's Nightly and STR from comment #0 works for me Thanks! Honza
You need to log in
before you can comment on or make changes to this bug.
Description
•