Opened 13 years ago

Closed 13 years ago

Last modified 12 years ago

#554 closed defect (fixed)

[patch]compressor renaming in function scope is order-dependant

Reported by: eric@… Owned by: jkuhnert
Priority: high Milestone:
Component: BuildTools Version: 0.2
Keywords: Cc: satish@…
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)

new_custom_rhino.diff (38.9 KB) - added by satish@… 13 years ago.
Patch to fix the issues with compressor dependency on order of declaration
alternat_custom_rhino_1_6R2.diff (34.9 KB) - added by guest 13 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 13 years ago by jkuhnert@…

Owner: changed from anonymous to jkuhnert@…

comment:2 Changed 13 years ago by bwalker@…

Updating with another example of the problem.

function Test(){

var caller = function(){

callee();

}

var callee = function(){

var x = "callee";

}

var caller2 = function(){

callee();

}

}

Results in:

function Test(){ var _1=function(){ callee(); }; var _2=function(){ var x="callee"; }; var _4=function(){ _2(); }; }

comment:3 Changed 13 years ago by satish@…

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 13 years ago by satish@…

Attachment: new_custom_rhino.diff added

Patch to fix the issues with compressor dependency on order of declaration

comment:4 Changed 13 years ago by satish@…

Cc: satish@… 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 13 years ago by James Burke

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 13 years ago by guest

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 13 years ago by guest

comment:7 Changed 13 years ago by jkuhnert

Owner: changed from jkuhnert@… to jkuhnert
Status: newassigned

comment:8 Changed 13 years ago by Adam Peller

Summary: compressor renaming in function scope is order-dependant[patch]compressor renaming in function scope is order-dependant

comment:9 Changed 13 years ago by jkuhnert

thanks adam .....applying now

comment:10 Changed 13 years ago by jkuhnert

Resolution: fixed
Status: assignedclosed

(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)

comment:11 Changed 12 years ago by (none)

Milestone: 0.4

Milestone 0.4 deleted

Note: See TracTickets for help on using tickets.