Opened 11 years ago

Closed 11 years ago

#8350 closed defect (invalid)

Build system not handling inlined comments correctly in some cases

Reported by: scott.a.jenkins Owned by: anonymous
Priority: high Milestone: tbd
Component: General Version: 1.2.3
Keywords: Cc:
Blocked By: Blocking:

Description

The following code snippet (which is legal javascript):

mojo.util = {

	/*private*/
	specifiedUrlExp: /^[a-z\.]*[:(0-9)]*\//i,
	
	/*public*/
	databaseUrl: function (/*string*/ arg, /*string*/ suffix) {
			//mojo.c.log('dominoUrl, arg=', arg);
			//mojo.c.log('expression=', mojo.util.specifiedUrlExp);
			
			var dummy = '';
									
			// Anything but non empty strings, return unmodified
			if (typeof arg !== 'string' || arg === '') {
				return arg;
			}

produces the following snippet in the compressed build file:

dojo._hasResource["mojo.util"]=true;dojo.provide("mojo.util");mojo.util={specifiedUrlExp:/^[a-z\.]*[:(0-9)]*\//i,databaseUrl:function(_2,_3)

Note that the inline comment beginning with a pair of slashes has been copied into the single line of Javascript in the compressed build file, thus commenting out the entire remainder of the build.

Change History (6)

comment:1 Changed 11 years ago by scott.a.jenkins

Clarification: the inline comment which is copied into the compressed file is the one on the third line of the source snippet, at the end of the regular expression, which I happened not to put a space before, that is "i".

comment:2 Changed 11 years ago by dante

I'm confused. the /\//i is legal because it is escaped and part of the regexp.

This line runs fine for me (single line):

dojo._hasResource["mojo.util"]=true;dojo.provide("mojo.util");mojo.util={ specifiedUrlExp:/^[a-z\.]*[:(0-9)]*\//i,databaseUrl:function(_2,_3){ console.log(false); } };mojo.util.databaseUrl();

your above "compressed" version doesn't show any additional comments, so maybe I'm missing something. are you saying the line mojo.c.log('foo') is being included in the build output? your paste doesn't show that (it stops at function(_2,_3), which is invalid (without {}'s).

comment:3 Changed 11 years ago by dante

this is what shrinksafe.dojotoolkit.org does to your above code (adding a } for function and } for object)

dojo.provide("mojo.util");mojo.util={specifiedUrlExp:/^[a-z\.]*[:(0-9)]*\//i,databaseUrl:function(_1,_2){var _3="";if(typeof _1!=="string"||_1===""){return _1;}}};

comment:4 Changed 11 years ago by dante

your above snippet when run through a build from trunk:

if(!dojo._hasResource["mojo.util"]){dojo._hasResource["mojo.util"]=true;dojo.provide("mojo.util");mojo.util={specifiedUrlExp:/^[a-z\.]*[:(0-9)]*\//i,databaseUrl:function(_1,_2){var _3="";if(typeof _1!=="string"||_1===""){return _1;} }};}

added a space in last } block to prevent trac from marking it up.

comment:5 in reply to:  4 Changed 11 years ago by scott.a.jenkins

My apologies, it appears that the bug, to the extent there is one, is actually in Microsoft Web Expression.

When I was reviewing the built file, Web Expression showed everything being commented out, and when I reviewed I saw the pair of slashes as the comment marker (which is how Expression interpreted them) when in fact that JavaScript? is treating the sequence as

(backslash to escape) (escaped slash being a part of the reg exp itself) (slash which closes the regexp) (i is part of the rexp saying it is case insignificant regexp)

I simply believed the syntax highlighting in Expression, and didn't analyze completely the entire character sequence previous. I probably shouldn't have done that (since I go back way before the days of syntax aware editors), but who questions their tools?

Again, I apologize. I think this one really is "no problem found," at least for Dojo.

comment:6 Changed 11 years ago by dante

Resolution: invalid
Status: newclosed

no worries - glad this isn't the issue (it would be a serious one if it were truly a bug) but something else is clearly going wrong with your building in XP, but I think that is a different ticket recently opened.

Note: See TracTickets for help on using tickets.