Page MenuHomePhabricator

Templates with parameters in the Index: namespace are dismissed when opening in editing
Closed, ResolvedPublic

Description

Yesterday, I added a table contents directly in the Index: namespace by using a template (Table) with some parameters. I previewed and saved, and it broke the whole page (see here and here).

By further searching, it happens the preview removed wikitext after the first "|". With further searching, the function getIndexEntries in includes/index/ProofreadIndexPage.php cuts after the first "\n|", so I removed the newline before the "|", I previewed, and saved, and it works (see here).

Event Timeline

Seb35 claimed this task.
Seb35 raised the priority of this task from to Medium.
Seb35 updated the task description. (Show Details)
Seb35 added a project: ProofreadPage.
Seb35 subscribed.

I just worked on this bug.

First I tried to use the Parser to obtain the wikitext of each argument, but I didn’t find a way to obtain it. My closest solution is:

$dom = self::getParser()->preprocessToDom( $text );
$frame = self::getParser()->getPreprocessor()->newFrame();
$dom = $dom->getFirstChild();
$childframe = $frame->newChild( $dom->getChildrenOfType( 'part' ) );
foreach( ... )
    $childframe->getArguments($arg)
    // and something like $frame->expand( $node, PPFrame::RECOVER_ORIG );

But I’m not confident if it will be the exact wikitext of the named argument.

So the second cheaper solution is to improve the regex in getIndexEntries. I tried and it fixes the bug. I submit the patch in a couple of hours.

Change 254687 had a related patch set uploaded (by Seb35):
Fix T119330 Templates with parameters in the Index: namespace are dismissed when opening in editing

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

Ok I just reworked on this and I guess I have the right solution.

$dom = self::getParser()->preprocessToDom( $text );
$frame = self::getParser()->getPreprocessor()->newFrame();
$dom = $dom->getFirstChild();
$childframe = $frame->newChild( $dom->getChildrenOfType( 'part' ) );
$output = '';
foreach ( $this->config as $varName => $property ) {
    if ( !array_key_exists( $varName, $childframe->namedArgs ) ) continue;
    $values[$varName] = $self::getParser()->getPreprocessor()->newFrame()->expand( $childframe->namedArgs[$varName], PPFrame::NO_ARGS | PPFrame::NO_TEMPLATES);
}

This is similar to what is done in Parser->getPreloadText(), but only the wikitext inside the named argument is given.

Ok I just reworked on this and I guess I have the right solution.

Amazing! Do you plan to work on a change implementing it?

Merci pour l’encouragement :) I just submitted this in the existing Gerrit change, but unfortunately some important details must still be improved.

Change 331643 had a related patch set uploaded (by Seb35):
[WIP] Use the parser to get each template parameter in the Index: namespace

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

Change 331643 abandoned by Seb35:
[WIP] Use the parser to get each template parameter in the Index: namespace

Reason:
Duplicate of previous Ic8ea89a42ad6d873284b927997c63866258c4ef4, I resubmit there.

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

Change 254687 merged by jenkins-bot:
Use the parser to get each template parameter in the Index: namespace

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

This is basically fixed thanks to Tpt and Tim Starling, as it can be seen on https://en.wikisource.beta.wmflabs.org/w/index.php?title=Index:Wind_in_the_Willows_(1913).djvu&diff=804&oldid=39 https://en.wikisource.beta.wmflabs.org/w/index.php?title=Index:Wind_in_the_Willows_(1913).djvu&diff=805&oldid=804 (I entered raw '|' and they were converted to {{!}}).

Not sure from where comes the transformation '|' -> '{{!}}' but anyway it works. Should the reverse transformation should be done when editing?

Had to revert the patch because it broken the creation of new pages on Wikisource and I didn't see any easy fix: T155682: Unable do create any page in Index namespace of wikisources.

Thanks for the revert and sorry for this. I guess it should be checked if there is a non-empty text and DOM, I will prepare a new version for this, but probably wait T155714 is fixed, it shouldn’t be too hard to write tests for that.

The change was integrated in the more general Content Model 'Index' in https://gerrit.wikimedia.org/r/#/c/328543/ (and I tested it specifically for this bug). Hence it is fixed, and should be live on Wikisource when it will be deployed.

If I understand correctly the deployments, the patch https://gerrit.wikimedia.org/r/#/c/328543/ is now live on Wikisource. However I just tested and it does not work: put the following wikitext in the 'Sommaire' of an index page, preview, and you see everything after the "\n|" (included) is removed.

{{Table
| page = {{pli|5}}
| indentation=-1
| titre = I. — Les temps.
}}

Sorry, the IndexContent class introduced in https://gerrit.wikimedia.org/r/#/c/328543/ is not used everywhere yet. https://gerrit.wikimedia.org/r/#/c/355883/ (review welcome) is using it in ProofreadIndexPage and should fix this issue.

@Seb35, @Tpt: All related patches in Gerrit have been merged or abandoned, so I assume this task is resolved.
(Please reopen via Add Action...Change Status in the dropdown menu if I am wrong.) Thanks!