Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#13431 closed enhancement (fixed)

Trailing comma leads to dojo-crash in IE7

Reported by: Franz Mayer Owned by: Rawld Gill
Priority: high Milestone: 1.7
Component: BuildSystem Version: 1.6.1
Keywords: Cc:
Blocked By: Blocking:

Description

When a trailing comma is in an object or class, this leads to crash dojo in IE7. It seems that IE8 also cannot load the specific resource, where the trailing comma occurs (but can load other files). It works fine with IE9 and Firefox.

Code-Example:

var sample = dojo.create(
		'div',
		{
			innerHTML: txt,
			style: {
				display: 'block',
				position: 'absolute',
				left: '0px',
				top: '0px',
				width: '100%',
				height: '100%',
			}
		},
		this.domNode,
		'first');

Sample code crashes dojo in IE7, because of the trailing comma.

Attachments (2)

buildUtils.js.diff (1.6 KB) - added by Franz Mayer 8 years ago.
Suggestion of solution in buildUtils.js
build.js.diff (1.5 KB) - added by Franz Mayer 8 years ago.
Changes in build.js

Download all attachments as: .zip

Change History (9)

Changed 8 years ago by Franz Mayer

Attachment: buildUtils.js.diff added

Suggestion of solution in buildUtils.js

Changed 8 years ago by Franz Mayer

Attachment: build.js.diff added

Changes in build.js

comment:1 Changed 8 years ago by bill

Resolution: invalid
Status: newclosed

Trailing commas are invalid javascript, and lead to exceptions in IE regardless of whether dojo is part of the page or not. Users should fix the syntax errors in their files.

comment:2 Changed 8 years ago by Franz Mayer

Resolution: invalid
Status: closedreopened

Of course it should be fixed by developers of the specific file, but it can be overseen very quickly, since the javascript compiler compiles everything without error. Or have I overseen any option of Rhino / Shrinksafe?

comment:3 Changed 8 years ago by Franz Mayer

When I have something like this piece of code

setTickerText: function(text,) {
// some code
}

the javascript compiler says correctly:

     [java] js: 	setTickerText: function(text,) {
     [java] js: ..............................^

But in the description-example it compiles fine (no error), even though there is a (invalid) comma before the curly brace.

comment:4 Changed 8 years ago by bill

severity: criticaltrivial

I think by "overseen very quickly" you mean that this type of syntax error can be "overlooked very easily". Yes, it's a common programming mistake. But, it can be caught by jslint or the syntax checker in webstorm, and probably by other syntax checkers too. Dojo also has a syntax/style checker in util/checkstyle that may catch this.

I'll leave it to Rawld to close this ticket again, unless he wants to enhance the builder to start checking for syntax errors.

comment:5 Changed 8 years ago by Rawld Gill

Milestone: tbd1.8
Status: reopenednew

Need to look at closure and rhino output to see if there is a way to easily given a user warning.

Marked for 1.8 because it's low priority and we need to get 1.7 out; naturally, I'll try to get it in 1.7.

comment:6 Changed 8 years ago by Rawld Gill

Resolution: fixed
Status: newclosed

Use the build switch optimize=closure and then inspect the report (named build-report.txt in the release dir by default). All syntax errors detected by closure will be reported in that file.

comment:7 Changed 8 years ago by bill

Milestone: 1.81.7
Note: See TracTickets for help on using tickets.