Wikimedia Bugzilla is closed!

Thanks for the careful and detailed report. There definitely is a syntax error in the file you attached, but I am not able to reproduce this corruption. (A video capture of my test session, using Opera 12 on Linux, is available at <https://saucelabs.com/tests/bc2c08bf827946f299f3476e319ae3f6>).

What happens if you clear localStorage but leave module storage enabled? That is, on mediawiki.org, run:

    localStorage.removeItem('MediaWikiModuleStore:mediawikiwiki');

And then reload the page several times to see if you can reproduce the issue.

Where did you get the code from? If you produced it using Opera's Dragonfly, then it might be borked, as it doesn't escape strings correctly when dumping them to console. If you haven't cleared local storage yet, can you copy the data from Dragonfly's "Storage → Local Storage" tab, key "MediaWikiModuleStore:mediawikiwiki"? (There shouldn't be any private data there.)

I got the code from Opera Dragonfly, by inspecting the variable data of $.globalEval when the error is caught and the script stopped (with the button "Display processing errors and stop when an exception occurs" on -- translation of this label from French, perhaps not exactly this in English), and I only changed the double quotes " to their escaped \" to make the variable a valid JS string for this report (I should have explain this process initially, sorry).

As Ori suggested, I cleared the MediaWikiModuleStore:mediawikiwiki item in localStorage and I have no more errors. So it is perhaps better to close this bug apart if someone wants to investigate more; perhaps some configuration or code changed since, and the code is now error-free in Opera. Anyway, thanks for the responses here.

WORKSFORME it is, then. Thanks for the report; we'll resurrect it if the issue ever comes up again.

Well, I just ran into this myself on en.wp.

I think I clicked "No" when Opera asked me whether I want to increase the local storage space available for en.wikipedia.org a few days ago. Could that be the cause?

Created attachment 14024 [details]
Value for the "MediaWikiModuleStore:enwiki" local storage key, copied per comment 2

I experienced again this bug on Commons. I worked around by using debug=true, so I keep the environment buggy in mode debug=false if you want I report stacks, variables, etc. Just ask me -- I don’t have ideas myself of tests which could be interesting for debugging.

Any idea to find the precise location of the syntax error? Given there is no bug in debug=true mode, I guess it might be related to the minification process.

In my case (attachment 14024 [details]), the problem is a missing 'function':

   …"jquery.suggestions@1385060185":"mw.loader.implement(
      \"jquery.suggestions\",\n(){(function($)…
                              ^
            a 'function' should appear right here and it'll work

Now what would cause that, I have absolutely no idea. Ghosts of misunderstood COBOL programmers?

I finally also found the error and it is the same as yours in another module (ext.uls.mediawiki for my two occurrences of the bug). I had previously difficulties to manage the heavy string and to properly split lines since Opera Dragofly changes newlines to spaces during the copy-paste of a variable.

This code is initially generated in RessourceLoader::makeLoaderImplementScript, and all seems normal; in particular, the $scripts variable do really begin with "function () {".

By investitating a bit more, I view as possible cause of the bug that Opera corrupted the string during some save (mw.loader.store.update:flush), possibly this was resolved/improved by Change I91d01d845a1a633414b94cc02e142a9956955c9d , althought this is not really convincing since I got my two errors in the really same place (same "typo"/same module) on two different wikis and you have the error on the same "typo" (and different module).

Given there was the "typo" in ext.uls.mediawiki in the localStorage of Commons, I cleared the localStorage and got no more error. Opera didn’t ask me to increase the storage size after the clearing and refreshing.

For information localStorage is between some bytes and 3.0 MB depending of the wikis.

I had the same JavaScript error for many days on multiple wikis. My solution: I had to delete the persistent storage (the so called "super cookies") in my Opera browser. Cleaning the cache was not enough. The scripts aren't cached any more in the normal browser cache, they are permanently stored in super cookies. I think I read this in a developers blog but can't find it at the moment.

The broken bracket was in the compressed version of the jquery.suggestions module.

Broken version (shortened by me):

mw.loader.implement("jquery.suggestions",
(){(function($){...}(jQuery));;},{"css":[...]},{});

Fixed version:

mw.loader.implement("jquery.suggestions",
function(){(function($){...}(jQuery));;},{"css":[...]},{});

I can't tell you why this happened. Nothing was changed in the module. Must have been an error in the compression that got fixed silently.

Suggested solution: Invalidate the broken module to force a reload in all browsers. I really hope this is possible.

(In reply to comment #10)
> I had the same JavaScript error for many days on multiple wikis. My
> solution: I had to delete the persistent storage (the so called
> "super cookies") in my Opera browser. Cleaning the cache was not
> enough. The scripts aren't cached any more in the normal browser
> cache, they are permanently stored in super cookies. I think I read
> this in a developers blog but can't find it at the moment.

For the record, this is accessible in UI via Preferences → Advanced →
Storage (you can view which sites store data in localStorage and how
much, and clear it per-site or entirely). You can also clear
localStorage for the "current" domain by calling
`localStorage.clear()` in Dragonfly's JS console.


> I can't tell you why this happened. Nothing was changed in the module. Must
> have been an error in the compression that got fixed silently.

I think it's pretty clear at this point that we're dealing with an
Opera bug; the question is whether it's worth trying to work around
it, or whether we should just blacklist Opera from RL localStorage.

Given the regularity in the examples posted so far, it seems that
String(function(){…}) sometimes returns a newline instead of the
'function' that should be at the beginning, this should be easy to
detect; I'd say we should try this first, and if the error doesn't
disappear, just blacklist Opera (which will cause it to load the
scripts normally, from browser cache or downloading them, and thus
result in a small-but-maybe-perceptible slowdown – or rather, lack of
the speedup). Ori, thoughts?


> Suggested solution: Invalidate the broken module to force a reload in all
> browsers. I really hope this is possible.

Yes, that's easily possible, either by bumping
$wgResourceLoaderStorageVersion (thanks to Ori's foresight :) ) or by
checking if `$.globalEval( concatSource.join( ';' ) )` in mediawiki.js
throws any SyntaxErrors and if yes, just gobble them up and continue
loading the scripts normally (from browser cache or actually
downloading them).

(In reply to comment #11)
> I think it's pretty clear at this point that we're dealing with an
> Opera bug

I'm not so sure. Why should Opera change a string? If this would be a browser bug I would expect something like "nction()" or "???????function()". But that's not what happens. It's a clean replacement of the first word "function" with a newline.

> seems that String(function(){…}) sometimes returns a newline

My suggestion then:

// Workaround for possible Opera bug. This line doesn't hurt other browsers.
s = s.replace(/^\s*\(/, 'function(');

> checking if `$.globalEval( concatSource.join( ';' ) )` in mediawiki.js
> throws any SyntaxErrors

I would love to see this in the code, no matter what other possible solution or workaround is implemented. There are endless possibilities why localStorage could be messed up. Never trust input coming from the browser.

(In reply to comment #12)
> I'm not so sure. Why should Opera change a string? If this would be
> a browser bug I would expect something like "nction()" or
> "???????function()". But that's not what happens. It's a clean
> replacement of the first word "function" with a newline.

So would I, but observations seem to prove us wrong :)

Debugging the assembly code seems like a lovely weekend project.
Well, maybe if the bug was at least always reproducible. Maybe
somebody, somewhere assigns NULL to `*p` instead of to `p` and causing
a memory leak, while also zeroing the first eight bytes instead of
clearing a pointer, and then later some sanity-checking code changes
the nulls to whitespace? We probably may never know.


> s = s.replace(/^\s*\(/, 'function(');

Yeah, that's basically what I had in mind.

Change 100930 had a related patch set uploaded by Bartosz Dziewoński:
mw.loader.store: More fault tolerance

https://gerrit.wikimedia.org/r/100930

Change 104149 had a related patch set uploaded by Bartosz Dziewoński:
mw.loader.store: Detect malformed function stringification

https://gerrit.wikimedia.org/r/104149

Change 104149 merged by jenkins-bot:
mw.loader.store: Detect malformed function stringification

https://gerrit.wikimedia.org/r/104149

## Similar handling for server error

To simulate the handling for invalid syntax from the server (instead of module storage):

* Change ResourceLoader::makeLoaderImplementScript():
  - 	$scripts = new XmlJsCode( "function () {\n{$scripts}\n}" );
  + 	$scripts = new XmlJsCode( "function () {throw new Error('Wee');\n{$scripts}\n}" );
* Set $wgResourceLoaderStorageEnabled = false; in LocalSettings.php
* Refresh page

Result:

  (i) Exception thrown by jquery.hidpi
  (i) [x] Error: Wee Error {stack: (...), message: "Wee"}
  > mw.loader.getState('jquery.hidpi')
    "error"
  > jQuery.byteLength
    undefined

## Steps to simulate bug with client error

* Visit page that will load module x (e.g. jquery.byteLength, loaded on every page by default in the Vector skin)
* Execute from console:

  var storeKey = mw.loader.store.getStoreKey();
  localStorage[storeKey] = localStorage[storeKey].replace('mw.loader.implement(\\"jquery.byteLength\\"', 'throw new Error(\\"infected module store\\"); mw.loader.implement(\\"jquery.byteLength\\"');

* Refresh page

Current result in console:

  [x] Uncaught Error: infected module store 
  > mw.loader.getState('jquery.byteLength')
    "loading" // indefinitely !
  > jQuery.byteLength
    undefined

After this patch:

  (i) Error while evaluating data from mw.loader.store
  (i) [x] Error: infected store Error {stack: (...), message: "infected store"} 

  > mw.loader.getState('jquery.byteLength')
    "ready"
  > jQuery.byteLength
    function (str) { ... }

Change 100930 merged by jenkins-bot:
mw.loader.store: Wrap script eval in try/catch

https://gerrit.wikimedia.org/r/100930

This should no longer be happening now.