#554 closed defect (fixed)
[patch]compressor renaming in function scope is order-dependant
Reported by: | Owned by: | jkuhnert | |
---|---|---|---|
Priority: | high | Milestone: | |
Component: | BuildTools | Version: | 0.2 |
Keywords: | Cc: | [email protected]… | |
Blocked By: | Blocking: |
Description
Using the compressor...
var my_obj = function() { this.my_method = function() { return _my_function(); } var _my_function = function() { return 'hi!'; } }
becomes
var my_obj=function(){ this.my_method=function(){ return _my_function(); }; var _1=function(){ return "hi!"; }; };
... so the constructor scoped-function "_my_function" is renamed "_1", but references to "_my_function" in methods defined before the function are not renamed "_1". Defining "_my_function" before it is referenced causes the compressor to work properly.
Attachments (2)
Change History (13)
comment:1 Changed 16 years ago by
Owner: | changed from anonymous to [email protected]… |
---|
comment:2 Changed 16 years ago by
comment:3 Changed 16 years ago by
I am the new soc student working on the JS Linker project. I was looking at this bug ealier and I believe I am very close to a good solution. I will update you guys when I have a patch for this issue.
Changed 16 years ago by
Attachment: | new_custom_rhino.diff added |
---|
Patch to fix the issues with compressor dependency on order of declaration
comment:4 Changed 16 years ago by
Cc: | [email protected]… added |
---|
Modification 1:
In org.mozilla.javascript.tools.shell.processFileSecure() method the input JavaScript? source file is read to create a Script object that represents a compiled script. But this is unnecessary because the Script object is already generated in the code earlier. So I have changed the method signature to directly pass this Script object instead of parsing of the source file to generate this Script object again.
Modification 2:
I have added functionality to org.mozilla.javascript.tools.shell.processOptions method to write the compressed file to a specified file using command options.
Example: To run the compressor on dojo.js and output dojo.js.compressed.js:
java -jar custom_rhino.jar -o dojo.js.compressed.js -c dojo.js
Modification 3:
I modified the compressScript method in org.mozilla.javascript.Context class to pass a newly generated parse tree of FunctionNodes? to the Decompiler class. This parse tree is used to generate a mapping for each Function node and parameters and variables names associated with it. This includes all the variables in the current function scope, other variables found while traversing the prototype chain until we reach the parent scope and variables found in the top-level scope. This mapping is used as a reference to check the parameters and variables names when generating new tokens for compression.
Modification 4:
I have made extensive modification to the org.mozilla.javascript.TokenMapper? class. I have inline comments explaining the modified methods in detail.
Tests
I have tested and confirmed that this patch works with examples provided to demonstrate this bug, dojo-0.2.2-kitchen_sink version and dojo-0.3.0-ajax version.
comment:5 Changed 16 years ago by
Milestone: | → 0.4 |
---|
Alex, did you write the original compressor code? If so, did you want to review Satish's patch (attached to this ticket, our SoC JS Linker student)? If not, I can take a look, but I'm not that familiar with the code, so it will be slow going.
comment:6 Changed 16 years ago by
Didn't see this and wrote a patch for ftp://ftp.mozilla.org/pub/mozilla.org/js/rhino1_6R2.zip (Note that it only modifies src/org/mozilla/javascript/Decompiler.java) Will attach in case somebody finds it useful.
Changed 16 years ago by
Attachment: | alternat_custom_rhino_1_6R2.diff added |
---|
comment:7 Changed 16 years ago by
Owner: | changed from [email protected]… to jkuhnert |
---|---|
Status: | new → assigned |
comment:8 Changed 16 years ago by
Summary: | compressor renaming in function scope is order-dependant → [patch]compressor renaming in function scope is order-dependant |
---|
comment:10 Changed 16 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
(In [5877]) Applied patch that fixes #554 thanks to the hard work from Satish (soc jslinker student).
While I was at it also upgraded to the latest rhino1_4 version + upgraded the requisite ant-dojotest task.
We should also now (if we didn't already ? ?) have better xml support in rhino, which I believe will make at least being able to create/test most widget operations..(with some exceptions I'm sure)
Updating with another example of the problem.
function Test(){
}
Results in:
function Test(){ var _1=function(){ callee(); }; var _2=function(){ var x="callee"; }; var _4=function(){ _2(); }; }