Page MenuHomePhabricator

Is there a better way for Translate to disable PST on messages?
Open, Needs TriagePublic

Description

In researching T236828: Unexpected PST effect when saving edits to .js pages that were formerly <nowiki> I turned up the (surprising?) fact that the Translate extension is the *only* user of the ArticlePrepareTextForEdit hook in core, and it only uses it to disable PST on articles in the message namespace.

Now that there are Content subclasses for all sorts of things, is there a better way to do this in modern MediaWiki?

I'd love to deprecate and remove this hook if possible.

Event Timeline

Interesting context, thanks. The Translate extension is installed on mediawikiwiki these days, but I guess $wmfTranslateUsePreSaveTransform is set to true universally.

My main point was that this was all back in 2011/2012, and we've had eight years of core dev since then. We have a MessageContent class now (actually introduced in late 2012!), and it actually disables the PST by default (it extends AbstractContent and AbstractContent has a no-op implementation of preSaveTransform()). It's possible the behavior of our wikis has changed since 2011 w/o anyone noticing, although MessageContent has a warning at the top about it not actually being the thing used for handling MediaWiki namespace content, so maybe MessageContent is actually just inconsistent w/ actual behavior.

Regardless, perhaps the more "modern" solution would be to have a TranslateWikiMessageContent subclass, or something like that, which disabled the PST, rather than hooking the parse process to hack the ParserOption. Or move the wgTranslateUsePreSaveTransform option into core as wgMessagesUsePreSaveTransform and remove the hook.

Having a hook that allows extensions to arbitrarily change ParserOptions seems way more powerful than is needed for this case. (See c6810d74d18437caff5a0af3efa4c42ec1811bb1 for instance.)

(Comment above edited, in case you're reading this over email; check phab for updates.)

I note that translatewiki still has $wgTranslateDisablePreSaveTransform = true; https://gerrit.wikimedia.org/r/plugins/gitiles/translatewiki/+/master/ExtensionSettings.php#57 which isn't even the name of the option (any more). In any case, it seems to confirm that TranslateWiki wants the PST disabled for messages. I don't think I've got enough admin permissions to check enwiki etc and see if they currently have PST enabled for messages or if the Content class refactoring over the past 8 years has quietly reverted to no-PST-on-Messages like MessageContent.php calls for.

Confirmed on my local wiki that PST still occurs on edits to the MediaWiki namespace, code in MessageContent notwithstanding.

Confirmed on my local wiki that PST still occurs on edits to the MediaWiki namespace, code in MessageContent notwithstanding.

MessageContent, is currently used only to show messages instead of page content. It's basically a hacky error reporting mechanism used by EditPage. I'm sorry for introducing this misleading class.

A separate content model for use in the MediaWiki namespace sounds tempting, but would be misleading: Messages can have several different content models (plain text, html, wikitex), and ther eare pages int he namespace that are not messages and have different models yet (CSS, JS, conversion tables).

In my mind, "message" should be a page type, not a content model. Whether PST is applied or not could be governed by the PageTypeHandler, see T209924: Specify PageTypeHandler.

Translate wants PST disabled for all messages and their translations (except message documentation!) to make sure what the translator inputs is not changed even if it happens to contain something that MediaWiki parser happens to process in PST. No-PST is mostly for non-MediaWiki translations actually.

It just happens that this no-PST also overlaps with NS_MEDIAWIKI, since some messages are translated there, and no-PST is coarsely applied per namespace level currently.

What makes sense from MediaWiki's point of view is, imho, a different question.

@Nikerabbit: @Krinkle wants more configurability of PST so that he can safely do bot edits of JS/CSS. If we added a per-user PST disable, in such a way that you could make that the default for all users on translatewiki, would that satisfy your need?

@daniel: thanks for that context and the link to the discussion on PageTypeHandler.

@Nikerabbit: @Krinkle wants more configurability of PST so that he can safely do bot edits of JS/CSS. If we added a per-user PST disable, in such a way that you could make that the default for all users on translatewiki, would that satisfy your need?

That sounds even more coarse than per namespace configuration. I think normal non-message page editing should continue to work normally, or we get some confused users wondering why something doesn't work there.

Mostly I'm trying to remove a hook that allows unrestricted ParserOptions mutation on any page or content type by extensions. Anything which is restricted to PST only is a step forward, even if it is coarser on other axes.

Mostly I'm trying to remove a hook that allows unrestricted ParserOptions mutation on any page or content type by extensions. Anything which is restricted to PST only is a step forward, even if it is coarser on other axes.

And to that end, @daniel -- putting in a new hook in Parser::preSaveTransform so that the test looked like;

if ( $options->getPreSaveTransform() && <hook didn't disable presave transform on $title>) {
  $text = $this->pstPass2( ... );
}

would make me happier as well, since it's the fact that ArticlePrepareTextForEdit has access to *every* ParserOption on *every* content type that makes me nervous. A hook like the one above would only affect PST and would only affect content types where PST was otherwise enabled.

Somewhat related to T299316 (w/r/t rendering JS as wikitext), T307691 (TOC generated for wikitext). A Content based way to disable TOC/PST on certain content types would be useful.

Adding missing Multi-Content-Revisions code project tag as Core Platform Team Initiatives (MCR) team tag is archived and its parent Platform Engineering team does not exist anymore

Tacsipacsi subscribed.

It has nothing to do with Multi-Content-Revisions. A possible outcome of this task could be the introduction of a new main-slot content type, not making use of multiple slots (which is what MCR is about).