Page MenuHomePhabricator

Reduce jqueryMsg parser overhead in jquery.accessKeyLabel
Closed, ResolvedPublic

Description

Following up on T202151, I notice there is still one unavoidable call to jquery.accessKeyLabel on all page loads, and it doesn't come from addPortletLink(). It comes from mediawiki.page.ready to fill in the tooltips for the navigation links already on the page from the server-side output.

Given it's unavoidable (requires browser information), I'd like to revisit the cause of its overhead.

In a nut shell, it's invoking the jqueryMsg wikitext-parser 4 times, which in turn has various DOM, jQuery, and other overhead.

Details

Related Gerrit Patches:

Event Timeline

Krinkle created this task.Aug 21 2018, 4:27 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 21 2018, 4:27 AM

It seems like these messages shouldn't require parsing given they contain just square brackets and either the empty string or a single space.

Unfortunately, it seems that messages cannot contain a single space due to messages being edited on translatewiki.net where they have to be saved with MediaWiki, which strips leading and trailing whitespace from a submitted revision, according to this message's documentation:

word-separator/qqq
languages/i18n/qqq.json:    "word-separator": "{{optional}}\nThis is a string which is (usually) put between words of the language. It is used, e.g. when messages are concatenated (appended to each other). Note that you must express a space as html entity   because the editing and updating process strips leading and trailing spaces from messages.\n\nMost languages use a space, but some Asian languages, such as Thai and Chinese, do not.",
..
languages/i18n/en.json: "word-separator": " ",
languages/i18n/eu.json: "word-separator": " ",
languages/i18n/fy.json: "word-separator": " ",

And that' the reason it requires the parser (in text normalisation mode, not complete html parsing).

I guess for the purposes of this case, we'll need to keep it, but we can at least cache it's result.

Change 454194 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] jquery.accessKeyLabel: Cache result of mw.msg() calls

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

Jdforrester-WMF added a subscriber: Jdforrester-WMF.

Maybe it'd be saner to make this a git-only translation string, and represent it directly as a " ", and the avoid the overhead?

@Jdforrester-WMF I'd like that very much. I don't know if there is a precedent of that and/or whether we're comfortable with enforcing that (and if so, whether to enforce technically and/or socially, and if technical, whether on twn side, mw side, or both?).

Messages flagged in TWN config (not in the qqq) as 'optional' are not visible on the site, and so are only editable via git. That might work?

Optional messages are editable, just hidden by default. Ignored messages are not editable (and will get removed from translation files).

It seems like these messages shouldn't require parsing given they contain just square brackets and either the empty string or a single space.
Unfortunately, it seems that messages cannot contain a single space due to messages being edited on translatewiki.net where they have to be saved with MediaWiki, which strips leading and trailing whitespace from a submitted revision, according to this message's documentation:

And that' the reason it requires the parser (in text normalisation mode, not complete html parsing).

It doesn't actually require any parsing. The   is replaced with a regular space in MessageCache::get(), before the message reaches the parsing/rendering code. I see that Krinkle changed his patch to just use mw.message().plain() (which does no parsing), I was going to suggest exactly that.

Maybe it'd be saner to make this a git-only translation string, and represent it directly as a " ", and the avoid the overhead?

Given the above, I don't think it's necessary, and there's not really a lot of overhead. The   in the source is only a minor annoyance to translators, in the very rare cases where someone needs to translate these optional messages. It's not a problem in MediaWiki's code.

Change 454194 merged by jenkins-bot:
[mediawiki/core@master] jquery.accessKeyLabel: Optimise by using mw.message().plain()

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

matmarex closed this task as Resolved.Aug 21 2018, 6:15 PM
matmarex assigned this task to Krinkle.
matmarex removed a project: Patch-For-Review.