Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Debug scope proxies don't let eval-in-frame code define new properties

RESOLVED FIXED in mozilla15

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Honza, Assigned: luke)

Tracking

Trunk
mozilla15
x86
Windows Vista
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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

5 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

5 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

5 years ago
Created attachment 628845 [details] [diff] [review]
fix and tests

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.
Assignee: general → luke
Status: NEW → ASSIGNED
Attachment #628845 - Flags: review?(jimb)

Comment 3

5 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

5 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

5 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

5 years ago
https://hg.mozilla.org/mozilla-central/rev/6b99d0e6e281
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(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
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.