Last modified: 2011-02-02 18:50:09 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T28265, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 26265 - Many semicolons are missing from JavaScript files
Many semicolons are missing from JavaScript files
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
UsabilityInitiative (Other open bugs)
unspecified
All All
: Normal minor (vote)
: ---
Assigned To: Trevor Parscal
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-12-06 10:50 UTC by Szőts Ákos
Modified: 2011-02-02 18:50 UTC (History)
5 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Szőts Ákos 2010-12-06 10:50:06 UTC
MediaWiki (1.16) is using lots of JS files and that's why I started minify and compress them. Compression is done by JavascriptPacker (http://joliclic.free.fr/php/javascript-packer/en/ ). But if more JS files are packed to one shorter form, browsers get allergic to the missing semicolons.

I started using the new editing interface (part of the UsabilityInitiative extension) and its JS files contained some syntax errors.

Please, put semicolons at the end of the following lines:
UsabilityInitiative/js/usability.js:
- 11

/plugins/jquery.async.js:
- 47
- 69
- 75
- Because this file is part of an other project I filed a bug report there also. Its URL: http://plugins.jquery.com/content/syntax-error-file-jqueryasyncjs

/plugins/jquery.textSelection.js:
- 168
- 180
- 192

/plugins/jquery.wikiEditor.js:
- 1430
- 1442
- 1454

/plugins/jquery.wikiEditor.templateEditor.js:
- 450
- 474

I don't know whether you have write access to the /skins/common directory, but if so, please do this with the other two files, and I won't open an other report for it:
/skins/common/ajaxwatch.js:
- 68

/skins/common/ajax.js
- 39
- 148

Thank you!

Btw, I know you are doing some type of work like this (ResoureLoader) and maybe you are interested in the following (I don't know to whom should I tell this).

After packing files, I got the following error:
(this.uiDialogTitlebarCloseText = $("<span/>")).addClass("ui-icon ui-icon-closethick").text(options.closeText).appendTo is not a function

It was a jQuery UI 1.7 bug and after upgrading to >1.8.6 this problem solved. However, in 1.8 the js2stopgap/ui.draggable.js will depend on ui.mouse.js so that should be loaded first.
Comment 1 Sam Reed (reedy) 2010-12-06 11:04:23 UTC
Thanks for the report

Doing a quick look against trunk/SVN head, a lot of these have been moved, and possibly fixed.

A lot of them come up to be comment lines, or even line numbers past the end of the file.

It's also quite likely a lot of them have been fixed in SVN, and then not backported, especially if they've come from third party libraries (ie if they're directly from jquery)

We do have access to everywhere, so fixing them isn't much of an issue :)

Even the phase3 ones, seemingly to be comments etc.


If you were to run it against a SVN copy, and report the errors, that'd be appreciated :)

I'll leave it open for Roan/Trevor to have a look at the latter issues for the moment :)
Comment 2 Szőts Ákos 2010-12-06 11:38:39 UTC
I use v1.16 so I really don't know where some files were moved but I tried my best and here is the result from the SVN trunk:

WikiEditor/modules…
contentCollector.js
- 434

ext.wikiEditor…
.addMediaWizard:
- 15: }); (the whole line)

.tests.toolbar:
- 245: setTimeout( function() { button.slideDown( 'fast' ); }, 2000 ); (the whole line)

jquery.wikieditor…
.dialogs:
- 38

.iframe:
- 1143
- 1155
- 1167
- 1313: }; } )( jQuery ); (the whole line)

.templateEditor:
- 256
- 381
- 626
- 650

.toc:
- 474: setTimeout( function() { $.wikiEditor.modules.toc.fn.unhighlight( context ); }, 1000 ); (the whole line)

.toolbar:
- 288
- 455
- 468
- 472

/skins/common/ajax.js:
- 36
- 65

/skins/common/ajaxwatch.js doesn't contain this error :)

If you can tell me now where the following files are located, I could inspect those also:
- UsabilityInitiative/js/usability.js
- /plugins/jquery.async.js
- /plugins/jquery.textSelection.js

I'm not sure, but as far as I know, only the jquery.async.js file is from a 3rd party which contains error.
Comment 3 Sam Reed (reedy) 2010-12-06 12:23:07 UTC
/skins/common/ajax.js:
- 36 - Is a } after "return true;"
- 65 - Is a } after "return A;"

If it's wanting a ; after those (which makes some sense), line 167 and 177 should also have them.

I've just added in r77872 to fix common/ajax.js

Looking at ajaxwatch.js, that already has those trailing ; at SVN head.

In SVN, most of the phase3 ones won't have been moved, I don't think.

For the Usability Extensions, they've been split down to separate extensions:
UsabilityInitative/WikiEditor -> WikiEditor


I'm interested to see how (or even, IF), the JS minifier Roan and Trevor are using do anything to correct this automatically (it's possible it might).

Though, it wouldn't be bad practise to do it manually.

Most of the other javascript stuff that's been folded into phase3, are in the resources directory (phase3/resources)
Comment 4 Bawolff (Brian Wolff) 2010-12-06 20:16:07 UTC
>I'm interested to see how (or even, IF), the JS minifier Roan and Trevor are
>using do anything to correct this automatically (it's possible it might).

Auto-semicolon-ing stuff sounds like something that would cause mysterious hard to find bugs. If you did have a smart algorithm for adding the semicolons, it'd probably be the same as the auto-semicolon insert rules in js, in which case there really wouldn't be much of a point.
Comment 5 Szőts Ákos 2010-12-06 20:22:55 UTC
No, I don't have one; I did the inspection semi-automatic.

I used Zend Studio which indicated me where syntax errors are. Maybe it worth a try to investigate Aptana, which is an open source software probably with a similar capability.
Comment 6 Sam Reed (reedy) 2010-12-06 22:31:35 UTC
My IDE found 192 in phase3...

Fixed those in r77922, r77923, r77924
Comment 7 Sam Reed (reedy) 2010-12-06 23:16:22 UTC
Another in r77926

And WikiEditor in r77928
Comment 8 Mark A. Hershberger 2011-01-31 19:58:25 UTC
Does this need any more work now that ResourceLoader is going in?
Comment 9 Sam Reed (reedy) 2011-01-31 20:12:06 UTC
(In reply to comment #8)
> Does this need any more work now that ResourceLoader is going in?

It's almost more needed now. The lack of semi colons when you have minified the files, cause more problems to the browsers (see original post)

I've probably got *most* of them, but likely not, so I'll poke them all and see how many more there seems to be...
Comment 10 Trevor Parscal 2011-01-31 21:25:38 UTC
ResourceLoader uses a minifier that's magic semi-colon safe. If there are cases where magic semi-colons are indeed breaking because of JavaScriptDistiller, that's a bug.
Comment 11 Mark A. Hershberger 2011-02-02 18:50:09 UTC
Closing this. New problems == new bugs.  Re Trevor in IRC:

<TrevorParscal> if someone finds a bug to do with magic semi-colons breaking,
                then it can be re-opened, it would be a valid regression

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links