Opened 13 years ago

Closed 12 years ago

#2730 closed defect (wontfix)

ShrinkSafe doesn't handle variable re-naming within 'eval' strings

Reported by: guest Owned by: alex
Priority: high Milestone:
Component: ShrinkSafe Version: 0.4.2
Keywords: Cc:
Blocked By: Blocking:

Description

ShrinkSafe? seems to be breaking code in the following scenario:

var currentElement = form.elements[i];
for (j=0; j < VALIDATION_TYPE.length; j++)
{
attributeIs = eval("currentElement."+VALIDATION_TYPE[j]) || currentElement.getAttribute(VALIDATION_TYPE[j]);

// do something
}

This code compresses to:

var _13=_10.elements[i];
for(j=0;j<VALIDATION_TYPE.length;j++){
attributeIs=eval("currentElement."+VALIDATION_TYPE[j])||_13.getAttribute(VALIDATION_TYPE[j]);
}

Where a local variable is used within a string in an 'eval' function, this variable is not renamed inside the string. In this example, the reference to currentElement will fail as this variable has been renamed to _13.

Obviously this is a tricky problem as we probably can't go searching the contents of eval statements for variable names to change. The possible solutions I see to this are:

  1. Do a rudimentary search in eval strings for variable names - if one is found, don't rename any local variables with that name. This is limited as it doesn't account for variable names inserted at runtime.
  1. Other compression tools around work around this by allowing a mechanism to prevent renaming specific variables (by special comments in the JavaScript? code for instance).

Apologies if I've missed some existing documentation somewhere on this problem, I can't seem to find anything on the Dojo site.

Change History (2)

comment:1 Changed 12 years ago by Adam Peller

Component: BuildToolsShrinkSafe

comment:2 Changed 12 years ago by alex

Resolution: wontfix
Status: newclosed

solving this in a reliable way is nearly impossible. Preventing renaming of variables in shrinksafe is straightforward: just make them public. Marking wontfix.

Note: See TracTickets for help on using tickets.