Page MenuHomePhabricator

[Regression] MediaWiki:Common.js not being loaded if it contains a syntax error
Closed, DeclinedPublic

Description

Hello,
Since around the Scribunto installation on http://fr.wikiversity.org, we've noticed that our many toggle bars were out of order.

We didn't change anything in the templates, Common.css and Common.js before the bug. Copying the same ones from Wikibooks or into my personal wiki shows that the current http://fr.wikiversity.org is the only site which doesn't work:

Error: JavaScript parse error: Parse error: Missing ; before statement in file 'MediaWiki:Common.js' on line 1149 @


Version: unspecified
Severity: major

Details

Reference
bz46426

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 1:37 AM
bzimport set Reference to bz46426.
bzimport added a subscriber: Unknown Object (MLST).

[Tentatively moving to the Scribunto component]

JackPotte: Thanks for reporting. Which toggle bars exactly? Could you attach a screenshot here please, and provide an URL where to see it?

Much more likely to be something changed in ResourceLoader that it now chokes on the unclosed comment at the end of their MediaWiki:Common.js. I can't see any possible way this could be related to Scribunto.

The "Error: JavaScript parse error: Parse error: Missing ; before statement in file 'MediaWiki:Common.js' on line 1149" message comes from includes/resourceloader/ResourceLoaderModule.php line 422.

I'm going to reassign this to ResourceLoader, where hopefully someone knows more about what might have made this start failing where it didn't before.

(In reply to comment #4)

Much more likely to be something changed in ResourceLoader that it now chokes
on the unclosed comment at the end of their MediaWiki:Common.js. I can't see
any possible way this could be related to Scribunto.

The "Error: JavaScript parse error: Parse error: Missing ; before statement
in file 'MediaWiki:Common.js' on line 1149" message comes from
includes/resourceloader/ResourceLoaderModule.php line 422.

I'm going to reassign this to ResourceLoader, where hopefully someone knows
more about what might have made this start failing where it didn't before.

Afaik this behaviour has not changed in ResourceLoader for over a year. If it did change, it seems like it fixed a bug, not introduced a bug.

An unclosed comment in javascript (not the validation by ResourceLoader, but when executing code in a real browser engine I mean) results in a "SyntaxError: Unexpected token ILLEGAL". Try executing the following in your browser's javascript console:

function foo() { return 4; }
alert(foo);
/* bla

Which means, if ResourceLoader hadn't detected this on the server side (it has a javascript parser that detects syntax errors like this) one of two things would've happened:

  1. It gets minified against something else which (if you're lucky) causes a big chunk of code to be incorrectly commented out until it finds a */ token somewhere that closes it. This will break code in weird and unexpected ways
  1. There is no future */ token, it is the last script in the row. It gets delivered and causes a fatal error. The entire script package (anything concatenated before this) is ignored and not executed.

Both are bad. What ResourceLoader does in this case:
Replace the code of the wiki page that caused the error with a line that logs the error to the console. That way everything else still works and a developer can find the error without breaking the site.

Again, afaik nothing changed and this never worked in the first place. If it did, it was likely by very lucky accident.

(In reply to comment #5)

Again, afaik nothing changed and this never worked in the first place. If it
did, it was likely by very lucky accident.

Either way, it's unlikely to change (at least not unless we figure out what made it work in the first place).

You now basically have a built-in linter, use it! I'd say just fix the syntax error and be done with it.