Change #1102316 had a related patch set uploaded (by Abijeet Patro; author: Abijeet Patro):

[mediawiki/extensions/Translate@master] Use a non special page as title when displaying diff

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

Change #1102316 merged by jenkins-bot:

[mediawiki/extensions/Translate@master] Use a non specialpage as title when creating MutableRevisionRecord

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

Appearing in other code paths too:

/w/api.php   Wikimedia\Assert\PreconditionException: Precondition failed: This Title instance does not represent a proper page, but merely a link target.
[stacktrace]
#0 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/title/Title.php(3816): Wikimedia\\Assert\\Assert::precondition()
#1 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/title/Title.php(3837): MediaWiki\\Title\\Title->assertProperPage()
#2 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/Revision/RevisionRecord.php(112): MediaWiki\\Title\\Title->toPageIdentity()
#3 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/Revision/MutableRevisionRecord.php(113): MediaWiki\\Revision\\RevisionRecord->__construct()
#4 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/parser/Parsoid/LintErrorChecker.php(79): MediaWiki\\Revision\\MutableRevisionRecord->__construct()
#5 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/preferences/SignatureValidator.php(268): MediaWiki\\Parser\\Parsoid\\LintErrorChecker->checkSome()
#6 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/preferences/SignatureValidator.php(124): MediaWiki\\Preferences\\SignatureValidator->checkLintErrors()
#7 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/preferences/DefaultPreferencesFactory.php(722): MediaWiki\\Preferences\\SignatureValidator->validateSignature()
#8 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/preferences/DefaultPreferencesFactory.php(256): MediaWiki\\Preferences\\DefaultPreferencesFactory->profilePreferences()
#9 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/preferences/DefaultPreferencesFactory.php(2111): MediaWiki\\Preferences\\DefaultPreferencesFactory->getFormDescriptor()
#10 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/api/ApiOptionsBase.php(118): MediaWiki\\Preferences\\DefaultPreferencesFactory->getResetKinds()
#11 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/api/ApiMain.php(1975): MediaWiki\\Api\\ApiOptionsBase->execute()
#12 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/api/ApiMain.php(943): MediaWiki\\Api\\ApiMain->executeAction()
#13 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/api/ApiMain.php(914): MediaWiki\\Api\\ApiMain->executeActionWithErrorHandling()
#14 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/api/ApiEntryPoint.php(153): MediaWiki\\Api\\ApiMain->execute()
#15 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/MediaWikiEntryPoint.php(201): MediaWiki\\Api\\ApiEntryPoint->execute()
#16 /srv/mediawiki/tags/2024-12-11_17:21:09/api.php(44): MediaWiki\\MediaWikiEntryPoint->run()
#17 {main}

Tagging Parsoid/Linter per the latest stack trace.

If we end up reverting/changing core to allow this (like it used to), then you can un-tag those again. If we end up chasing down the violations and requiring them to accomodate each diff with a "can-exist" page title, then the next place to fix would be Parsoid/Linter.

Given the end-of-year timeline and this week being the last train of the year, it may be prudent to revert and chase these down in the new year, to avoid unnecessary breakage. I am off most of the day today.

Change #1104736 had a related patch set uploaded (by Subramanya Sastry; author: Subramanya Sastry):

[mediawiki/core@master] Revert "Revision: make RevisionRecord::getPage() return a value object"

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

In T381982#10398472, @Nikerabbit wrote:

Appearing in other code paths too:

/w/api.php   Wikimedia\Assert\PreconditionException: Precondition failed: This Title instance does not represent a proper page, but merely a link target.
[stacktrace]
#0 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/title/Title.php(3816): Wikimedia\\Assert\\Assert::precondition()
#1 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/title/Title.php(3837): MediaWiki\\Title\\Title->assertProperPage()
#2 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/Revision/RevisionRecord.php(112): MediaWiki\\Title\\Title->toPageIdentity()
#3 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/Revision/MutableRevisionRecord.php(113): MediaWiki\\Revision\\RevisionRecord->__construct()
#4 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/parser/Parsoid/LintErrorChecker.php(79): MediaWiki\\Revision\\MutableRevisionRecord->__construct()
#5 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/preferences/SignatureValidator.php(268): MediaWiki\\Parser\\Parsoid\\LintErrorChecker->checkSome()
#6 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/preferences/SignatureValidator.php(124): MediaWiki\\Preferences\\SignatureValidator->checkLintErrors()
#7 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/preferences/DefaultPreferencesFactory.php(722): MediaWiki\\Preferences\\SignatureValidator->validateSignature()
#8 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/preferences/DefaultPreferencesFactory.php(256): MediaWiki\\Preferences\\DefaultPreferencesFactory->profilePreferences()
#9 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/preferences/DefaultPreferencesFactory.php(2111): MediaWiki\\Preferences\\DefaultPreferencesFactory->getFormDescriptor()
#10 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/api/ApiOptionsBase.php(118): MediaWiki\\Preferences\\DefaultPreferencesFactory->getResetKinds()
#11 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/api/ApiMain.php(1975): MediaWiki\\Api\\ApiOptionsBase->execute()
#12 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/api/ApiMain.php(943): MediaWiki\\Api\\ApiMain->executeAction()
#13 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/api/ApiMain.php(914): MediaWiki\\Api\\ApiMain->executeActionWithErrorHandling()
#14 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/api/ApiEntryPoint.php(153): MediaWiki\\Api\\ApiMain->execute()
#15 /srv/mediawiki/tags/2024-12-11_17:21:09/includes/MediaWikiEntryPoint.php(201): MediaWiki\\Api\\ApiEntryPoint->execute()
#16 /srv/mediawiki/tags/2024-12-11_17:21:09/api.php(44): MediaWiki\\MediaWikiEntryPoint->run()
#17 {main}

What's going on here is that someone is trying to change their signature. The SignatureValidator uses Parsoid+Linter to check that the new signature is valid (doesn't have lint errors). We construct a MutableRevisionRecord in the process, and end up crashing when that MutableRevisionRecord is expected to represent a PageIdentity aka "an editable page". But of course we're parsing a user signature, we don't have an "editable page"! What "context page title" are we supposed to set here?

This is a case of T278376: Parser Use of RevisionRecord for a page that can't exist: (eg Special:MyLanguage/Main Page), and I think the assumption that the MutableRevisionRecord must represent a PageIdentity (that is, "an editable page") instead of a PageReference ("a viewable page") is at fault here. The parser is invoked many times where the context is "a viewable page" not "an editable page", and it uses MutableRevisionRecord as a pair type to couple a page title with a revision id (always 0 in the case of a non-editable page, but often 0 when the page is editable but not created etc as well).

This is a fundamental mismatch in page modeling (https://www.mediawiki.org/wiki/Manual:Modeling_pages).

In T381982#10406068, @Krinkle wrote:

Tagging Parsoid/Linter per the latest stack trace.

If we end up reverting/changing core to allow this (like it used to), then you can un-tag those again. If we end up chasing down the violations and requiring them to accomodate each diff with a "can-exist" page title, then the next place to fix would be Parsoid/Linter.

It's not checking "can exist" (that's PageReference) it is checking "can edit" (PageIdentity) and that is fundamentally incompatible with the way the parser is used (eg on Special pages).

Change #1104736 merged by jenkins-bot:

[mediawiki/core@master] Revert "Revision: make RevisionRecord::getPage() return a value object"

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

@abi_ on what wiki did this happen? Or more importantly, with what version of MediaWiki? Is this from production? Or from CI?

Looks like it was on translatewiki.net, the version was presumably master as of 2024-12-11 13:29:32.

Both the revert and the patch made it to MW-1.44-notes (1.44.0-wmf.8; 2024-12-17) and the breaking change was only in that branch. That thus never made it to Wikimedia production. That was apparently caught on translatewiki (which runs out of the master branch), was fixed in time and thus is no more blocking the train ;)

This likely needs a follow-up discussion. I suggest to have it on another place/task for clarity.

In T381982#10411763, @hashar wrote:

Both the revert and the patch made it to MW-1.44-notes (1.44.0-wmf.8; 2024-12-17) and the breaking change was only in that branch. That thus never made it to Wikimedia production. That was apparently caught on translatewiki (which runs out of the master branch), was fixed in time and thus is no more blocking the train ;)

Ah, I see. I was very confused by the patch breaking this but not fixing T380677. This explains it!

Fun fact: the issue with SignatureValidator wasn't caught in CI because it only happens if the main page is a special page. SignatureValidator uses the main page pas parsing context, which usually works fine... but on translatewiki.net it doesn't.

In T381982#10414167, @daniel wrote:

Fun fact: the issue with SignatureValidator wasn't caught in CI because it only happens if the main page is a special page. SignatureValidator uses the main page pas parsing context, which usually works fine... but on translatewiki.net it doesn't.

Using the main page as a placeholder title should be declared as an anti pattern?

In T381982#10414903, @Nikerabbit wrote:

Using the main page as a placeholder title should be declared as an anti pattern?

It's not grate, but it's IMHO better than using a special page. It's a bit annoying that a page is required as context for parsing at all. It should be optional.

When validating a signature, we need *some* page as context. The user's talk page would be a logical choice, but then you'd have to supply a user to the vlidator.

In T381982#10414167, @daniel wrote:

Fun fact: the issue with SignatureValidator wasn't caught in CI because it only happens if the main page is a special page. SignatureValidator uses the main page pas parsing context, which usually works fine... but on translatewiki.net it doesn't.

Hmm, I can't find where SignatureValidator does it. It seems to instead hardcode SpecialPage::getTitleFor( 'Preferences' ) in several places as the context title.

In T381982#10414903, @Nikerabbit wrote:

Using the main page as a placeholder title should be declared as an anti pattern?

I agree that it's an anti-pattern. Other code sometimes special-cases the main page in interesting ways. For example, MobileFrontend used to apply special transformations to it (T32405).

If you use a random page name in the main namespace, you will also have trouble on wikis where the main namespace is not wikitext, like Wikidata. (I just noticed that this was the workaround in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Translate/+/1102316/5/src/Synchronization/MessageWebImporter.php, that might be bad.)

In T381982#10415480, @daniel wrote:

When validating a signature, we need *some* page as context. The user's talk page would be a logical choice, but then you'd have to supply a user to the vlidator.

The user’s talk page can also be problematic, as links to the user talk page would turn into self-links, which could make checkUserLinks() fail.

In T381982#10415607, @matmarex wrote:

In T381982#10414167, @daniel wrote:

Fun fact: the issue with SignatureValidator wasn't caught in CI because it only happens if the main page is a special page. SignatureValidator uses the main page pas parsing context, which usually works fine... but on translatewiki.net it doesn't.

Hmm, I can't find where SignatureValidator does it. It seems to instead hardcode SpecialPage::getTitleFor( 'Preferences' ) in several places as the context title.

rMW includes/parser/Parsoid/LintErrorChecker.php uses $this->titleFactory->newMainPage().