Ticket #1242 (closed Bug: fixed)

Opened 15 months ago

Last modified 4 months ago

Comments are not always stripped when compressing

Reported by: fredck Owned by: fredck
Priority: Normal Milestone:
Component: Project : FCKpackager Version:
Keywords: Confirmed Review+ Cc:

Description

The following input:

if ( /*is.ns5 &&*/ subDivs[idx].className=='DLG_edittext' && subDivs[idx].style.width ) // fix firefox bug
    button152.enable(bMode&&bEdit/*TODO &&theApp.clipboard.canPaste(theApp.clipboard.titleData.title.arChld[0])*/);  //

var testFunc = function(tempNode) { 
  var fs = tempNode.style.fontSize; 
  fs = parseInt(fs); //.substr(0,fs.length-2); // remove px
  fs = Math.round( fs / 1.3333 ) + "pt"; // convert to points
  var bs = fs * 2; 
  return bs 
}

Outputs like this:

if (/*is.ns5&&*/ subDivs[idx].className=='DLG_edittext' && subDivs[idx].style.width ) //fix firefox bug button152.enable(bMode&&bEdit/*TODO&&theApp.clipboard.canPaste(theApp.clipboard.titleData.title.arChld[0])*/);  //var testFunc=function(A) {var B=A.style.fontSize;B=parseInt(B);B=Math.round(B / 1.3333 ) + "pt"; //convert to points var C=B*2;return C};

The comments have not been stripped out, breaking the code in some cases.

Attachments

1242.patch (0.6 KB) - added by fredck 5 months ago.
1242_2.patch (1.1 KB) - added by fredck 5 months ago.

Change History

Changed 9 months ago by w.olchawa

  • keywords Confirmed added

Changed 5 months ago by fredck

Changed 5 months ago by fredck

  • keywords Review? added

The problem here is in the regex used to isolate and save strings and regular expressions. It was not looking for the "*" char before "/" in the regular expression start.

There is also a big problem with the division literal (/). In the following example:

var a = 1 ; b = 1 ; g = 1 ;
var c = a / b / g ;

The processor was considering / b / as a regular expression.

There would be possible to solve this problem, but PHP doesn't support flexible length on lookbehind assertions. So, the patch also includes two further assertions that impose some rules for regular expressions and divisions:

  • There must be a semicolon at the end of lines containing regular expressions. It helps us avoiding the problem in the following line:
fs = Math.round( fs / 1.3333 ) + "pt"; // convert to points
  • There must be a space after the division character. So, "(((a / b / g)))" is considered a division. If instead we have "a/b/g", "/b/" is considered a regular expression.

Changed 5 months ago by fredck

Changed 5 months ago by fredck

Opps... I've wrongly reverted some changes before creating the first patch. The new one should be ok.

Changed 5 months ago by fredck

  • owner set to fredck
  • status changed from new to assigned

#1343 must be fixed together with this one, otherwise our code will get broken.

Changed 4 months ago by martinkou

  • keywords Review+ added; Review? removed

Changed 4 months ago by fredck

  • status changed from assigned to closed
  • resolution set to fixed

Fixed with [2237].

Changed 4 months ago by fredck

The rule of requiring lines containing regexes to be terminated by ; does not apply to cases like the following:

if ( /regex/.test( '' ) )

So, [2237] broke our code. I've fixed it with [2242], so now only division literals are required to be followed by spaces.

Note: See TracTickets for help on using tickets.