Last Comment Bug 758499 - Remove JSRESOLVE_WITH
: dev-doc-complete
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
-- normal (vote)
: mozilla16
Assigned To: Jeff Walden [:Waldo] (remove +bmo to email)
: Jason Orendorff [:jorendorff]
Depends on:
  Show dependency treegraph
Reported: 2012-05-24 20:48 PDT by Jeff Walden [:Waldo] (remove +bmo to email)
Modified: 2012-06-06 08:45 PDT (History)
1 user (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch (2.55 KB, patch)
2012-05-25 11:08 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
dmandelin: review+
Details | Diff | Splinter Review

Description User image Jeff Walden [:Waldo] (remove +bmo to email) 2012-05-24 20:48:52 PDT
JSRESOLVE_WITH was added long ago in bug 463997 as a hackaround for some oddness with |with (window) var x = 17;|.  The details of exactly went wrong are never *quite* fully expressed in the bug with an exactly minimal browser testcase, alas, but that was the expressed gist of it.

With the changes in bug 632003 and bug 577325, however, I suspect the hackaround that JSRESOLVE_WITH implemented to be no longer necessary -- we define vars and names declared by function statements rather than set them.  Thus I expect it's no longer necessary to special-case window expandos when being set for vars inside |with|, and it's no longer necessary to special-case with-based lookups via the resolve flag mechanism.

And indeed, if I remove JSRESOLVE_WITH and run JS tests, the test mentioned in bug 463997 as failing does not fail!  Let's do this.

...after seeing if embedders (sigh) are using this.  Given the extreme obscurity of it and that it was added only to placate Gecko, I am cautiously optimistic.  Thread started:!topic/
Comment 1 User image Jeff Walden [:Waldo] (remove +bmo to email) 2012-05-25 11:08:10 PDT
Created attachment 627295 [details] [diff] [review]
Comment 2 User image Jeff Walden [:Waldo] (remove +bmo to email) 2012-05-25 16:13:32 PDT
Comment on attachment 627295 [details] [diff] [review]

Review of attachment 627295 [details] [diff] [review]:

Feel free to wait however long you feel is right for embedder feedback (I only posted last night, so this is still a bit fresh).  But note that this flag was never even documented anywhere, so the odds anyone but us used it, and will comment in the newsgroup, are slim to none.
Comment 3 User image Jeff Walden [:Waldo] (remove +bmo to email) 2012-05-25 16:15:50 PDT is all green so far with a J result or two on every platform, fwiw.
Comment 4 User image Jeff Walden [:Waldo] (remove +bmo to email) 2012-06-05 10:03:23 PDT \o/
Comment 5 User image Jeff Walden [:Waldo] (remove +bmo to email) 2012-06-05 10:13:33 PDT
I documented the presence of JSRESOLVE_WITH, and its obsoletion, here:

Also I added a note about its removal to the 1.8.8 notes:
Comment 6 User image Ed Morley [:emorley] 2012-06-06 08:45:08 PDT

Note You need to log in before you can comment on or make changes to this bug.