Page MenuHomePhabricator

ULS: JSON loading problems (Access-Control-Allow-Origin)
Closed, ResolvedPublic

Description

On: https://www.wikidata.org/wiki/Wikidata:Main_Page

[2x] XMLHttpRequest cannot load https://bits.wikimedia.org/static-1.21wmf2/extensions/UniversalLanguageSelector/lib/jquery.uls/i18n/en.json. Origin https://www.wikidata.org is not allowed by Access-Control-Allow-Origin.

[2x] XMLHttpRequest cannot load https://bits.wikimedia.org/static-1.21wmf2/extensions/UniversalLanguageSelector/i18n/en.json. Origin https://www.wikidata.org is not allowed by Access-Control-Allow-Origin.

ULS: Unknown language als. load.php:235
ULS: Unknown language bat-smg. load.php:235
ULS: Unknown language fiu-vro. load.php:235

[16x] XMLHttpRequest cannot load https://bits.wikimedia.org/static-1.21wmf2/extensions/UniversalLanguageSelector/i18n/en.json. Origin https://www.wikidata.org is not allowed by Access-Control-Allow-Origin.

  • The extension documentation[1] contains nothing about requiring cross-domain privileges to work. If this is a requirement, then it must be documented and properly configured on Wikimedia (which it isn't right now)
  • It repeatedly fails for the same stuff. 2x 2x for jquery.uls's en.json and a whopping 16 shots at the main en.json file. Whatever caching mechanism is in place, is broken. It probably doesn't cover for the fact that ajax is asynchronous (storing result objects instead of promises/deferreds)
  • 3 requests for unknown languages

And all this when merely viewing an English page in an English browser without being logged-in before having touched anything on the page.

Should it be requesting all that data all the time? I suppose the trigger label in the top corner is provided by the MediaWiki component, it should defer further init until if/when needed.

[1] https://www.mediawiki.org/wiki/Extension:UniversalLanguageSelector


Version: master
Severity: normal
Whiteboard: code-quality
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=45047

Details

Reference
bz41489

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 12:51 AM
bzimport set Reference to bz41489.
bzimport added a subscriber: Unknown Object (MLST).

Bug 41454 tracks the 404s for non-localized(also failed requests) locales.

I could ask where is the documentation that says $wgExtensionAssetsPath (or whatever is used here) might be on another domain.

We need the Allow-Origin anyway for geoiplookup.

Wikidata added to wgCrossSiteAJAXdomains

(In reply to comment #2)

I could ask where is the documentation that says $wgExtensionAssetsPath (or
whatever is used here) might be on another domain.

That's been a given since the start. Also, cross-domain shouldn't be an issue since CORS isn't supported by all browsers.

We need the Allow-Origin anyway for geoiplookup.

No, not at all. How is that relevant? The Wikimedia geoiplookup uses javascript (not JSON) in a <script> tag, which isn't subject to Allow-Origin (just like loading jQuery from a CDN).

The geoiplookup that ULS uses by default uses JSONP (not cross-domain JSON, but JSONP, which isn't JSON but javascript calling a function).

This issue is not resolved, the 3 problems still exist (one of which is now resolved in modern browsers):

  • Shouldn't require CORS.
  • Shouldn't request the same json file 16 times.
  • 3 requests for unknown languages (bug 41454).

Remaining issue is the one of allow-origin. Bug 25886 seems to be related to that.

Also added docs https://www.mediawiki.org/wiki/Extension:UniversalLanguageSelector#Configuration

What else needs to be done?

(In reply to comment #6)

Remaining issue is the one of allow-origin. Also added docs.
What else needs to be done?

Could somebody answer this? Nikola / Krinkle?

Well, bug 25886 isn't fixed yet, so this bug would still occur.

However I'd argue that we don't need ops to implement CORS on bits (bug 25886).

ULS shouldn't be using CORS to load JSON data from static files. It should use JSON-P, preferably from the API.

(In reply to comment #8)

Well, bug 25886 isn't fixed yet, so this bug would still occur.

However I'd argue that we don't need ops to implement CORS on bits (bug 25886).

ULS shouldn't be using CORS to load JSON data from static files. It should use
JSON-P, preferably from the API.

Or from ResourceLoader.

(In reply to comment #8)

Well, bug 25886 isn't fixed yet, so this bug would still occur.

However I'd argue that we don't need ops to implement CORS on bits (bug 25886).

ULS shouldn't be using CORS to load JSON data from static files. It should use
JSON-P, preferably from the API.

.. because CORS is only supported on modern browsers (ULS is currently breaking violating WMF browser compatibility guidelines), and it is a PITA to set up on caching servers (hence bug 25886).

(In reply to comment #8)

ULS shouldn't be using CORS to load JSON data from static files. It should use
JSON-P, preferably from the API.

As far as I know it is jquery.i18n which is loading the files. We are not going to change the i18n file format for this, so the ULS extension would need to implement custom loading wrapper for both backend and frontend.

Or perhaps we could just skip the effort and load the static files from the current domain ignoring bits altogether?

This has been "highest priority" for five weeks now.
Does anybody actively work on this, or shall I bump this down?

I am waiting for further feedback on the two alternatives discussed in comment #11.

(In reply to comment #11)

(In reply to comment #8)

ULS shouldn't be using CORS to load JSON data from static files. It should use
JSON-P, preferably from the API.

As far as I know it is jquery.i18n which is loading the files. We are not
going
to change the i18n file format for this, so the ULS extension would need to
implement custom loading wrapper for both backend and frontend.

Or perhaps we could just skip the effort and load the static files from the
current domain ignoring bits altogether?

The wrapper sounds like the way to go. A simple ResourceLoader or API module. Which ever works best for you.

API:

Implement:

GET /api.php?action=<module>&lang=<lang>&callback=..
...({...data from json file..});

Load:

$.getJSON(api, function (data) {
  mw.ext.uls.provideLang(data);
});

ResourceLoader:

Implement:

register module (if you only ever need 1 lang per page, switch getScript response based on context language within 1 module, or register 1 module per language)
module's getScript would be like mw.ext.uls.provideLang({...data from json file..});

Load:

mw.loader.load( '<module>.<lang>' );

The API approach is probably better. It gives more flexibility in usage (by providing the data directly from the callback) and doesn't fill the startup module with 1 lot of modules for every language code.

Niklas: Requested feedback provided in comment 14.
Do you either plan to fix this, or instead lower priority? Thanks.

Niklas: This is highest priority for quite some weeks now. Do you have a potential assignee in mind? Could you answer comment 15?

I'm not planning to work on this in nearest future unless if it assigned to me in a sprint planning.

Decreased priority. Nothing's on fire.

(In reply to comment #14)

(In reply to comment #8)
Or perhaps we could just skip the effort and load the static files from the
current domain ignoring bits altogether?

The wrapper sounds like the way to go. A simple ResourceLoader or API module.
Which ever works best for you.

Krinkle, is there a problem if we directly load the static json files from the current domain? What is the advantage of having a wrapper(RL or API) that reads the json files and serve here?

Krinkle: ping - Could you answer comment 19 please?

API was written to load the json in I37638e06f21ab573c9ce37a4e9fb20bc763ac98f.