Page MenuHomePhabricator

substituting {{#tag:ref}} tags and templates; and pipe tricks fail in Page namespace
Closed, ResolvedPublicBUG REPORT

Description

List of steps to reproduce (step by step, including full links if applicable):

Similarly for pipe tricks
https://en.wikisource.org/w/index.php?title=Page:Sandbox.djvu/1&diff=prev&oldid=11564812

Standard naepsace
Both compared to https://en.wikisource.org/wiki/Sandbox
https://en.wikisource.org/w/index.php?title=Wikisource%3ASandbox&type=revision&diff=11564818&oldid=11537868

What happens?:
No substitution takes place, so nothing happens when it should. Substituting the like that in main namepsace works as expected

What should have happened instead?:

{{subst:#tag:ref| &hellip; }} => <ref> &hellip; </ref>

Substituting templates also fails in the Page: ns

Event Timeline

DannyS712 triaged this task as Unbreak Now! priority.EditedAug 5 2021, 5:04 AM
DannyS712 subscribed.

Assuming this is due to the train, tagging as a blocker

Note that in the show changes display before saving it appears to substitute, but upon actually saving it doesn't.

Billinghurst renamed this task from substituting {{#tag:ref}} fails in Page namespace to substituting {{#tag:ref}} and pipe tricks fail in Page namespace.Aug 5 2021, 5:06 AM
Billinghurst lowered the priority of this task from Unbreak Now! to Needs Triage.
Billinghurst updated the task description. (Show Details)

Assuming this is due to the train, tagging as a blocker

Note that in the show changes display before saving it appears to substitute, but upon actually saving it doesn't.

While I definitely want fix now, as it is going to hold up my work, I am not certain that it blocks everything outside of WS. I haven't looked at other custom namespaces.

Billinghurst triaged this task as Unbreak Now! priority.Aug 5 2021, 5:08 AM
Billinghurst updated the task description. (Show Details)

Assuming this is due to the train, tagging as a blocker

Note that in the show changes display before saving it appears to substitute, but upon actually saving it doesn't.

While I definitely want fix now, as it is going to hold up my work, I am not certain that it blocks everything outside of WS. I haven't looked at other custom namespaces.

Its likely going to affect other extensions too - I was going to update my prior comment, but I'll put here instead:
Suspected cause: refactoring of Content::preSaveTransform into ContentHandler and ContentTransformer service, rMWb782a7e66ddf: Move Content::preSaveTransform to ContentHandler and T287156: Move Content::preSaveTransform method that don't belong in Content to ContentHandler, that task lists a number of other extensions that were not updated yet either

Assuming this is due to the train, tagging as a blocker

Note that in the show changes display before saving it appears to substitute, but upon actually saving it doesn't.

In Page: ns I do not see that it works in preview, I see broken in preview, and broken on save.

Billinghurst renamed this task from substituting {{#tag:ref}} and pipe tricks fail in Page namespace to substituting {{#tag:ref}} tags and templates; and pipe tricks fail in Page namespace.Aug 5 2021, 5:18 AM
Billinghurst updated the task description. (Show Details)

Assuming this is due to the train, tagging as a blocker

Note that in the show changes display before saving it appears to substitute, but upon actually saving it doesn't.

In Page: ns I do not see that it works in preview, I see broken in preview, and broken on save.

I have the Show previews without reloading the page preference enabled, turning that off resulted in broken preview and show changes too, so I assume thats why. Is the code for generating the wikitext for the live preview different from that which is used to generate the wikitext for non-live preview?

The problem is also present when trying to use substitution to quickly change the capitalization of the text, i.e. {{subst:uc:text}} and {{subst:lc:TEXT}} fail to work. As far as I have tested this seems to be limited to the Page: namespace on the French Wikisource, with Talk pages being unaffected.

Change 710288 had a related patch set uploaded (by Ppchelko; author: Ppchelko):

[mediawiki/core@master] Support deprecated Content::preSaveTransform override

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

The root cause of this is clear. We are transitioning PST from Content to ContentHandler, so that we can use dependency injection for PST. In the transition period, when some extensions still override Content::preSaveTransform and some extensions already override ContentHandler::preSaveTransform, we have tacky code that detects whether the extension has overridden PST in Content, and call that.

However, we didn't put the detection code in all the core-defined of ContentHandler subclasses. PageContent for ProofreadPage extends TextContent, so PageContentHandler ends up inheriting TextContentHandler::preSaveTransform, that was not detecting that PageContent defines custom preSaveTransform

Change 710288 merged by jenkins-bot:

[mediawiki/core@master] Support deprecated Content::preSaveTransform override

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

I overlooked this task during all wiki deployment today and we're now at group2. I see a merged fix but no backport to 1.37.0-wmf.17. Do I need to rollback group2/block the train or can we backport/sync this fix?

Change 710102 had a related patch set uploaded (by 20after4; author: Ppchelko):

[mediawiki/core@wmf/1.37.0-wmf.17] Support deprecated Content::preSaveTransform override

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

How widespread is the impact of this bug remaining unfixed? If it's a nitch use and we can fix it with a backport in the short term then I wouldn't roll back. On the other hand, if this is causing some fallout elsewhere or triggering major production breakage then definitely roll back.

It seems like the patch applies cleanly to the branch. If we can get someone to code review this cherry-pick then I'd say this can just be deployed as a backport.

How widespread is the impact of this bug remaining unfixed? If it's a nitch use and we can fix it with a backport in the short term then I wouldn't roll back. On the other hand, if this is causing some fallout elsewhere or triggering major production breakage then definitely roll back.

I guess this bug only affects the advanced editors using the {{subs: feature and should not trigger any breakage for reading the wiki and for most edits. Rolling the backport seems the best thing to do imho.

Mentioned in SAL (#wikimedia-operations) [2021-08-05T20:23:36Z] <dduvall> 1.37.0-wmf.17 promoted to all wikis. no new errors or concerning rates (T281158). fixes for open UBN T288191 will be handled via backport (see task discussion)

I guess this bug only affects the advanced editors using the {{subs: feature …

Don’t forget about the pipe trick. That’s bound to bite all users and at a higher proportion of edits than subst.

Change 710102 merged by jenkins-bot:

[mediawiki/core@wmf/1.37.0-wmf.17] Support deprecated Content::preSaveTransform override

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

Mentioned in SAL (#wikimedia-operations) [2021-08-05T22:11:17Z] <jforrester@deploy1002> Synchronized php-1.37.0-wmf.17/includes/content/ContentHandler.php: T288191: Support deprecated Content::preSaveTransform override (1/2) (duration: 01m 00s)

Mentioned in SAL (#wikimedia-operations) [2021-08-05T22:12:26Z] <jforrester@deploy1002> Synchronized php-1.37.0-wmf.17/includes/content/: T288191: Support deprecated Content::preSaveTransform override (2/2) (duration: 00m 55s)