Closed
Bug 343765
Opened 18 years ago
Closed 18 years ago
Function defined in a let statement/expression doesn't work correctly outside the let scope
Categories
(Core :: JavaScript Engine, defect, P2)
Core
JavaScript Engine
Tracking
()
VERIFIED
FIXED
mozilla1.9alpha1
People
(Reporter: nanto, Assigned: mrbkap)
References
()
Details
(Keywords: verified1.8.1)
Attachments
(3 files, 1 obsolete file)
10.48 KB,
patch
|
brendan
:
review+
|
Details | Diff | Splinter Review |
745 bytes,
text/plain
|
Details | |
2.10 KB,
patch
|
brendan
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; ja; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060706 Minefield/3.0a1 If a function contains variables boud by a let statement/expression, the variables' values change after leaving the let scope. Reproducible: Always Steps to Reproduce: js> let (a = 2, b = 3) { function f() { return [a, b]; } f(); } 2,3 js> f(); Actual Results: function () { return [a, b]; },[object global] Expected Results: 2,3 This bug blocks such a operation: var divs = document.getElementById("div"); for (let i = 0; i < divs.length; i++) divs[i].onclick = let (div = divs[i]) function () { alert(div.id + " is clicked."); };
Assignee | ||
Comment 1•18 years ago
|
||
Ugh, what does 'at top level' mean anyway?
Assignee: general → mrbkap
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Priority: -- → P2
Hardware: PC → All
Target Milestone: --- → mozilla1.9alpha
Reporter | ||
Comment 2•18 years ago
|
||
Sorry, I made typos. > If a function contains variables boud by a let statement/expression, variables bound by > var divs = document.getElementById("div"); var divs = document.getElementsByTagName("div");
Assignee | ||
Comment 3•18 years ago
|
||
We needed to clear the block objects' private data's and update their slots with the current values when popping blocks (both in the normal JSOP_LEAVEBLOCK case and in JSOP_SETSP for exceptions). Brendan and I also noticed that the condition for breaking out of the scopechain popping loop was incorrect, the block depth needed to be strictly less than the new stack depth to stay on the scope chain.
Attachment #228328 -
Flags: superreview?(shaver)
Attachment #228328 -
Flags: review?(brendan)
Comment 4•18 years ago
|
||
Comment on attachment 228328 [details] [diff] [review] Fix, v1 Kind of a joint patch, so shaver's sr= is more meaningful here. And Mike wrote JSOP_SETSP, long ago (and we jointly updated it to handle with scope popping belatedly ;-). /be
Attachment #228328 -
Flags: review?(brendan) → review+
Comment on attachment 228328 [details] [diff] [review] Fix, v1 sr=shaver
Attachment #228328 -
Flags: superreview?(shaver) → superreview+
Assignee | ||
Comment 6•18 years ago
|
||
The previous patch missed the case where returning from the midst of a block (let expression, etc.) would fail to clean up the block objects on the scope chain.
Attachment #228328 -
Attachment is obsolete: true
Attachment #228343 -
Flags: superreview?(shaver)
Attachment #228343 -
Flags: review?(brendan)
Comment 7•18 years ago
|
||
Comment on attachment 228343 [details] [diff] [review] Fix, v2 r=me again! /be
Attachment #228343 -
Flags: review?(brendan) → review+
Comment 8•18 years ago
|
||
The v2 patch emerged when I pointed out the return environment-capturing function from block case, and mrbkap had been nicely writing up testcase in a single file. We need those attached here so bc can harness them. /be
Assignee | ||
Comment 9•18 years ago
|
||
Fix checked into trunk.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 10•18 years ago
|
||
Reporter | ||
Comment 11•18 years ago
|
||
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060706 Minefield/3.0a1 Build ID: 2006070614 The result of URL-testcase is still incorrect.
Assignee | ||
Comment 12•18 years ago
|
||
Sorry, I didn't see the testcase in the URL field. It's an easy fix, though: we can't store the result in the stack before we call js_PutBlockObject.
Assignee | ||
Updated•18 years ago
|
Attachment #228351 -
Flags: review?(brendan)
Assignee | ||
Updated•18 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 13•18 years ago
|
||
Comment on attachment 228351 [details] [diff] [review] Oops, missed that one I was looking on, again. Should have seen this last time. Shaver, send your tachyons! /be
Attachment #228351 -
Flags: review?(brendan) → review+
Assignee | ||
Comment 14•18 years ago
|
||
Oops, this was checked in at 2006-07-06 15:46.
Status: REOPENED → RESOLVED
Closed: 18 years ago → 18 years ago
Resolution: --- → FIXED
Comment 15•18 years ago
|
||
(In reply to comment #14) > Oops, this was checked in at 2006-07-06 15:46. Blake, it looks like this was fixed on 1.8 due to the js1.7 landing. If that is not correct, please remove the fixed1.8.1 kw. Thanks. Checking in regress-343765.js; /cvsroot/mozilla/js/tests/js1_7/block/regress-343765.js,v <-- regress-343765.js initial revision: 1.1
Flags: in-testsuite+
Keywords: fixed1.8.1
Comment 16•18 years ago
|
||
verified fixed 1.8.1, trunk 20060723 win/linux/mac(ppc|tel)
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.1 → verified1.8.1
Attachment #228343 -
Flags: superreview?(shaver)
You need to log in
before you can comment on or make changes to this bug.
Description
•