Wikimedia Bugzilla is closed!

A few comments:
1. UI strings are needed for the tooltip of the clickable elements, so (some) strings must be available.
2. I thought this was cached <somehow>. If not, should be.

Question:
3. Is this new behavior, or has this behavior changed recently? If it is not new, it should have been like this since we implemented L10n loading through the API instead if directly loading the JSON.

Removed "see also" to bug 50391, as that was duplicate requests in the same page load. This is not about that. This issue is about repeated requests, where repeated loads of the same data between page view should probably be avoided.

I also notice that the "action=ulslocalization" API call goes to the site (nl.wikipedia.org) instead of bits.wikimedia.org. Not sure is we can/should make that call to come from bits.

(In reply to comment #2)
> I also notice that the "action=ulslocalization" API call goes to the site
> (nl.wikipedia.org) instead of bits.wikimedia.org. Not sure is we can/should
> make that call to come from bits.

bits.wikimedia.org can't serve API calls.

Couple quick drive-by notes:

* If most page views only need one tooltip, that should probably be bundled as a message in the main ResourceLoader module...

* Can the whole thing be redone as a ResourceLoader module (which *would* be served from bits), or is the format of the API calls locked in in some way that prevents that? It looks like it doesn't rely on anything user- or session-specific.

* I do see some public caching headers being set in the source:

		$this->getMain()->setCacheMode( 'public' );
		$this->getMain()->setCacheMaxAge( 300 );

however I don't know if all those make it through to the outside.

(In reply to comment #1)
> 3. Is this new behavior, or has this behavior changed recently? If it is not
> new, it should have been like this since we implemented L10n loading through
> the API instead if directly loading the JSON.

I don't think it's new. I pointed it out before on other bugs about ULS being slow, but no one was bothered.

(In particular I pointed it out in bug 49935 comment 17.)

The API vs RL usage for this was discussed in https://bugzilla.wikimedia.org/show_bug.cgi?id=41489#c14

Please read bug 41489 for more information why we ended up with the current solution.

Points to consider:
* In jquery.uls we cannot have dependencies on MediaWiki components.
* In jquery.uls (via jquery.i18n) we have a custom, json-based i18n file format.
* Serving that file directly from bits is not possible because of cross-origin restrictions.
* We are not going to change the file format from json to anything executable.
* We implemented API module which just servers the files as-is with some caching.
* The API module does not have version based cache invalidation.
* Doing this via resource loader module would mean that we need to wrap or reformat the json data into JavaScript on the fly and implement support for that in jquery.i18n.

It's not obvious to me which action(s) should be taken here. Some options:
1) Delay the api call until ULS trigger is clicked. This needs some refactoring moving the trigger related tooltip message*s* from json to i18n.php file.
2) Improve the caching of the api call
3) Use resource loader. I currently have no idea how this would look in practice.

Hopefully we will have clarity on this on Monday. Have a great weekend.

(In reply to comment #6)
> (In particular I pointed it out in bug 49935 comment 17.)

This is a different thing, and like stated on that bug, the code you pointed out is not executed in our environment. Instead of "I told you so" I'd like to see us all work together to design and implement a good solution.

(In reply to comment #9)
> This is a different thing, and like stated on that bug, the code you pointed
> out is not executed in our environment.

It is not now. It used to be until I pointed it out.

I did not mean to be all "told you so", I just pointed to an earlier discussion about the same topic (we're now talking partially about the code written to fix that bug, as far as I understand it).


(In reply to comment #8)
> * In jquery.uls we cannot have dependencies on MediaWiki components.

I'll dare to say that yes, you can and should if it means making the performance more acceptable, even if it means maintaining a fork. (And to be honest, the division of ULS into various modules has looked artificial to me ever since I learned it exists.)


> * Serving that file directly from bits is not possible because of
> cross-origin restrictions.

Can't you serve it as JSONP? Can't you have CORS set up so that the restrictions go away?

(In reply to comment #8)
> Please read bug 41489 for more information why we ended up with the current
> solution.
> 
> Points to consider:
> * In jquery.uls we cannot have dependencies on MediaWiki components.
> * In jquery.uls (via jquery.i18n) we have a custom, json-based i18n file
> format.
> * Serving that file directly from bits is not possible because of
> cross-origin
> restrictions.
> * We are not going to change the file format from json to anything
> executable.
> * We implemented API module which just servers the files as-is with some
> caching.
> * The API module does not have version based cache invalidation.
> * Doing this via resource loader module would mean that we need to wrap or
> reformat the json data into JavaScript on the fly and implement support for
> that in jquery.i18n.

This is what EventLogging does (schemas are JSON); see includes/ResourceLoaderSchemaModule.php in the EventLogging repository. JSON is already JavaScript, so to convert a JSON blob into something executable all you need to do is to wrap it in a function invocation. See the getScript module in that file.

> 
> It's not obvious to me which action(s) should be taken here. Some options:
> 1) Delay the api call until ULS trigger is clicked. This needs some
> refactoring moving the trigger related tooltip message*s* from json to
> i18n.php file.

- Wrap the code in an RL module, a la EventLogging.
- Defer loading until the page has finished loading OR the hover event on the cog is triggered. You don't have to wait for a user click, but you don't have to block the page either.

> 2) Improve the caching of the api call

Not an option, IMO. I don't have hard numbers, but if I recall correctly Yahoo! found that 30-40% of requests were always uncached, even with aggressive caching, presumably due to people resetting their browsers, using public kiosks, etc. A full HTTP request is really expensive. If you use ResourceLoader, you can fetch the JSON data and JS / CSS resources in a single request (which I think we'll want to do -- it's not just the messages that can be deferred but some of the JS/CSS too), and you'll benefit from localStorage when that gets rolled out.

> 3) Use resource loader. I currently have no idea how this would look in
> practice.

Something like this:

	class JsonMessagesModule  extends ResourceLoaderModule {
		function getDependencies() {
			return array( 'ext.uls.moduleWithInitFunc' );
		}

		function getModifiedTime( ResourceLoaderContext $context ) {
			return filemtime('/path/to/json');
		}

		function getScript( ResourceLoaderContext $context ) {
			$json = file_get_contents('/path/to/json');
			return Xml::encodeJsCall( 'ext.uls.initFunc', array( $json ) );
		}
	}


> Hopefully we will have clarity on this on Monday. Have a great weekend.

Yep! I think this is very solvable, so no sweat. Have a great weekend all.

(In reply to comment #4)
>         $this->getMain()->setCacheMode( 'public' );
>         $this->getMain()->setCacheMaxAge( 300 );

As a quick fix, this should be set to 31536000 (one year), not 300 (five minutes). Don't worry about invalidation since we'll move this to ResourceLoader anyway.

Change 93175 had a related patch set uploaded by Ori.livneh:
Set max-age to 2419200 (one month) for action=ulslocalization API calls

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

Change 93175 merged by jenkins-bot:
Set max-age to 2419200 (one month) for action=ulslocalization API calls

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

Change 93447 had a related patch set uploaded by Nikerabbit:
ResourceLoader Module for serving json based localization messages

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

Change 93447 merged by jenkins-bot:
ResourceLoader Module for serving json based localization messages

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

Given comment 16, what's the status of this bug?

The above patch was merged and already deployed in mediawiki.org. Does not show any api hits.

Change 95484 had a related patch set uploaded by Ori.livneh:
Update UniversalLanguageSelector to b2f9e4211e

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

Change 95484 merged by Ori.livneh:
Update UniversalLanguageSelector to b2f9e4211e

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