wikimedia-gadgets / twinkle

The English Wikipedia twinkle javascript helper
http://en.wikipedia.org/wiki/Wikipedia:Twinkle
Other
135 stars 147 forks source link

Replace all uses of wgMonthNames and wgMonthNamesShort #723

Closed Amorymeltzer closed 4 years ago

Amorymeltzer commented 4 years ago

Not super urgent yet, but these will be removed from mw.config down the line. We should discuss how we'd like to proceed.

Our best options seem to be either:

  1. Revert #635, where we removed our own built-ins, or
  2. Require loading mediawiki.language.months and then use the mw.language.months.names and mw.language.months.abbrev arrays
Amorymeltzer commented 4 years ago

Option 2 is preferable if only for l10n reasons, so what's another dependency amongst friends?

siddharthvp commented 4 years ago

Option 2 is not good because as noted on phab, mw.language uses the language of the user than that of the wiki. That would be quite counter-intuitive.

We have a couple of more options:

  1. We are already loading moment. We could use moment()._locale._months and moment()._locale._monthsShort. But this is not guaranteed to give the local month names on all wikis. For it to work, the library needs to have locale registered. It works on fr and de WPs (both of which have both en and the local language locale registered), but not on es, zh, and hi, which only have the en locale registered. (Run moment.locales() to see list of registered locales.)

  2. Another option is to just use the API (fr example). A single api call can get all month names and short month names. This works correctly on every wiki. But then, that's a preemptive API call on every page load :( But until recently, an api call was preemptively being used on every page load (a33a3c5) to check EC userright, added by @MusikAnimal. So it maybe it isn't an issue after all?

I guess the best options are 1 and 4. I'm leaning on 1 as we aren't accounting for other wikis' usage anywhere else in Twinkle. But since this is morebits, it might be a worthy consideration here.

Amorymeltzer commented 4 years ago

I suppose it depends on how (if?) they do it I suppose — I would hope the wiki-language V user-language issue to be something they resolve before removing these. If not, then you're right and 2 is out and 1 is probably our best bet — i18n stuff is, as you say, not a concern atm and when (if?) that ever happens, month names will be the least of the worries.

Funny you should mention moment... I want to play around with some things and see if we can't remove it.

siddharthvp commented 4 years ago

Funny you should mention moment... I want to play around with some things and see if we can't remove it.

the moment().calender() function ("Yesterday at 2:14 PM") can't be implemented easily. Though we could just remove that and display absolute dates. But I wouldn't do that. Its a cheap library loaded efficiently via the resourceloader. And it makes date handling a bliss, as well as less error-prone, and is free of browser/es-version quirks potentially associated with JS Date. We could even think of replacing all instances of JS Date and use moment exclusively.

Amorymeltzer commented 4 years ago

Better discussed elsewhere, though. If I get a moment this weekend I'll start a discussion.

At any rate, the ExC check was rightly removed, so I wouldn't say it's a model to be emulated if we can avoid it.

siddharthvp commented 4 years ago

Just saying: we could do something like this. The response is small enough, and will be cached by the browser. Since this response is will never change over time, we can throw in a large max-age and the immutable headers to request the browser to read from the cache and not do unnecessary api re-validations. How are month names generally handled in i18n applications, just hard-coded in the localisation files?

MusikAnimal commented 4 years ago

IE 11 apparently supports Date.prototype.toLocaleString() (minus timezones), so we could do something like https://stackoverflow.com/a/39972988/604142. Regardless, nothing else in Twinkle is localized, so I don't see a need to make API calls or complicate the code.

siddharthvp commented 4 years ago

Date.prototype.toLocaleString()

That's interesting. But you can't expect native stuff to work reliably for all languages across different platforms. On my system, for instance, it doesn't work for ceb, so, sa, ur, among the few checked. It appears to work only for popular languages.

nothing else in Twinkle is localized

I have plans to change that. #747 and #758 are a few preliminary steps.

Amorymeltzer commented 4 years ago

A tad offtopic but I missed this earlier:

How are month names generally handled in i18n applications, just hard-coded in the localisation files?

In my (very limited experience), yes (e.g. rails-i18n), although everything I've seen hasn't tried to contend with two different languages at once.

MusikAnimal commented 4 years ago

But you can't expect native stuff to work reliably for all languages across different platforms. On my system, for instance, it doesn't work for ceb, so, sa, ur, among the few checked. It appears to work only for popular languages.

A bit surprising. I wonder if you need the language itself installed on your system.


The system I generally see for localizing gadgets is to have the messages for each language as a separate JS page (hopefully one day JSON can be behind ResourceLoader). The gadget definition then pulls from the root language first (English), then checks for the one for the wgUserLanguage and merges it in. This is how I did it for MoreMenu, and HotCat among others work the same way:

This way you only load the messages that are needed (target and fallback), and you can keep localization in one place (Meta, most likely) so that other wikis importing Twinkle don't have to localize if the work has already been done.

For messages that already exist in MediaWiki, I think importing them via ResourceLoader is the right approach, which is what option 2 is about. Or, mw.Api.plugin.messages (preferred over querying the API directly, as with option 4). I would argue the user's language is what you'd want for localization, since this is how MediaWiki behaves (except for things stored in wikitext, e.g. signature timestamps). E.g. I am a global-sysop and I want to block a user on Hindi Wikipedia, it'd be nice if I could use English since I don't know Hindi.

In all cases, you can cache these messages with mw.storage.session. This avoids redundant API calls within the same session, and doesn't pollute the normal local storage with masses amounts of data.

At the Technical Conference two weeks ago, we talked about a better system for global gadgets. I won't get into that here, except to say that a solution is still a good ways off, so I think we can move forward with our own solution for now. Just make sure you're calling msg() (or something similar) for all messages, so that we can switch out the localization backend whenever the time comes.

siddharthvp commented 4 years ago

I would argue the user's language is what you'd want for localization

Twinkle edits, unlike MediaWiki or MoreMenu. We need the month names in Twinkle for making the section headers for user warnings and in CSD/PROD logs, and for determining XFD log page names ("WP:AFD/2019 January 18"). This obviously needs the wiki's content language rather than the user's language.

Or, mw.Api.plugin.messages (preferred over querying the API directly, as with option 4).

We don't have mediawiki.api declared as a dependency!

At the end of the day, I think we've have to scratch all this and go back to option 1. There are a handful of default status and API error messages in morebits that anyway need to be translated, the month names can go along with that.

MusikAnimal commented 4 years ago

Twinkle edits, unlike MediaWiki or MoreMenu. We need the month names in Twinkle for making the section headers ...

Bah, of course! What about magic words such as {{subst:CURRENTMONTHNAME}}? This I believe uses the content language. There are other issues about formatting (some wikis may want "[year] [month]"), which we can revisit later.

My other comments were in thinking ahead about localization. Short-term, if magic words don't do it, then yes option 1 seems best and sticks with the status-quo of not offering localization.

MusikAnimal commented 4 years ago

What about magic words such as {{subst:CURRENTMONTHNAME}}?

Silly me, we need to scan the page to find out what heading to put the warning under.

I've got other ideas, but I'll save that for a discussion about localizing Twinkle. For now I concur option 1 is best.

Krinkle commented 4 years ago

I found through code search that a number of gadgets and user scripts are using wgMonthNames and wgMonthNamesShort for detecting dates in user signatures. These are generally in the content language, which means such code is likely already not working correctly when the user's language is different from the content language.

For a few such cases, the much easier fix is to remove the config call in favour of an inline array with the values based on the current content language (example diff), or where the code actually requires the English words (example diff).

The use in morebits.js is quite generic on the Date prototype, so it's hard to determine what it is used for. If it really needs to be based on the user language, then mediawiki.language or Moment is a sensible migration target.

Otherwise, a hardcoded array, API call, or {{subst:}} are all good options as well.

Krinkle commented 4 years ago

The first phase of T219340 is to remove wgMonthNamesShort. The other one (wgMonthNames) will still take a few more weeks/months to be migrated away from in WMF-maintained code bases.

I'm currently looking at user scripts and gadgets and migrating away from wgMonthNamesShort where possible. Twinkle/morebits is currently one of a few where I'm not confident on an obvious replacement so will track this issue instead. Let me know how I can help :)

Amorymeltzer commented 4 years ago

Thanks for the note @Krinkle — I'm actively monitoring the phab, but having a clearer timeline is quite helpful! I dothink the move from wiki-language to user-language (as noted above and at phab) is notable, and I'm kind of surprised that mw.language doesn't have both in there.


I think we're all in agreement that option 1 (revert #635) is best for now? To wit, I've opened #796. Beyond that, though, if our concern is l10n, this is clearly the least of our worries, and among the easiest to deal with (anyone using morebits has hopefully long-since accounted for it).

I'm happy to leave this open to keep track of work on a better path forward, although I'm beginning to think that (after #796) this'd be a good thing to move into your Morebits.date @siddharthvp!