Page MenuHomePhabricator

Templates with parameters in the Index: namespace are dismissed when opening in editing
Open, MediumPublic

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).

Details

Related Gerrit Patches:

Event Timeline

Seb35 created this task.Nov 22 2015, 1:22 PM
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 added a subscriber: Seb35.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 22 2015, 1:22 PM
Seb35 added a comment.Nov 22 2015, 1:29 PM

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

Seb35 added a comment.Aug 1 2016, 3:41 PM

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.

Tpt added a subscriber: Tpt.Aug 1 2016, 5:43 PM

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?

Seb35 added a comment.Aug 1 2016, 10:24 PM

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.

Seb35 added a comment.Jan 19 2017, 3:30 PM

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.

Seb35 added a comment.May 19 2017, 3:11 PM

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.

Seb35 added a comment.Jun 3 2017, 9:44 AM

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.
}}
Tpt added a comment.Jun 3 2017, 9:48 AM

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.