Page MenuHomePhabricator

Double-parse in parser function results
Closed, ResolvedPublic

Description

Author: wikipedista

Description:
Using {{{vars}}} in the else text of a ParserFunctions #if causes a bug, in which both the "then" and the else text are showed.

#if syntax and example:
{{ #if: <condition> | <then text> | <else text> }}
{{ #if: {{{parameter|}}} | Parameter is defined. | Parameter is undefined, or empty }}

For example, lets say that I want to change the text of a template depending on the number of variables set. In this case, if {{{2}}} is defined the text will be in the plural and both variables {{{1}}} and {{{2}}} will be showed.
* {{ #if: {{{2|}}} | The articles {{{1}}} and {{{2}}} are nice. | The article {{{1}}} is nice. }}

The problem is that, because of the use of variables in the else text the #if will output the following:

  • The article The articles {{{1}}} and {{{2}}} are nice. is nice.

A duplication of the bug may be seen on the sandbox at Wikipedia (EN): http://en.wikipedia.org/w/index.php?title=Wikipedia:Sandbox&oldid=49532948

Best regards,


Version: unspecified
Severity: major
URL: http://en.wikipedia.org/w/index.php?title=Wikipedia:Sandbox&oldid=49532948

Details

Reference
bz5678

Event Timeline

bzimport raised the priority of this task from to High.Nov 21 2014, 9:11 PM
bzimport set Reference to bz5678.
bzimport added a subscriber: Unknown Object (MLST).

wikitech-l-54398 wrote:

I have tried this on the latest SVN checkout, and it doesn't happen, although it
still happens on Wikipedia, so this has presumably been fixed.

omniplex wrote:

"then clause clobbers {{{1}}}" is the issue reported in
[[m:Talk:Parser functions]] as "Odd bevaviour: {{{1}}} in {{#if:}}".

My test page [[m:User:Omniplex/Pathoschild]] still shows
it, so if it's already fixed then it's not yet installed
on Meta. A variant is "else clause clobbers {{{2}}}".

omniplex wrote:

[[m:Talk:ParserFunctions#New_5678_test_results]] shows that
this bug is less harmful than I feared, and also predictable.

omniplex wrote:

5678 affects also "plural:", it's not limited to the new parser functions.
Example given on [[m:ParserFunctions]] below "known problems". So far all
"bugs" related to ParserFunctions turned out to be either features or more
general oddities like this one. Could somebody please reclassify 5678 ?

omniplex wrote:

New observation, 5678 does NOT affect #ifeq:,
for examples see

http://meta.wikimedia.org/wiki/Help:Substitution#Corrupted_default_value

PITA, I was about to document that 5678 is at
least completely predictable, but apparently
#ifeq: somehow gets something right where
similar "colon functions" with more than two
parameters have serious (wrt subst:) issues.

omniplex wrote:

Please ignore comment #5, hallucination on my side, it's 100% predictable for
all "colon functions", and it can even hit named parameters. So "#ifeq:" is no
special case, it's only less likely to get it wrong with "#ifeq:".

This is an example what might happen:

if a template contains following code:

  • {{ #if: {{{1}}} | {{{2}}} | {{{1}}} - {{{1}}} article {{{1}}} is nice. }}

and is called by:
{{template|foo}}

result will be:

  • foo - foo article foo is nice.

possible solution to the problem

when I was reading on meta page for wikimedia extension, I found that the
functions could return an array instead of a string, and that array could take
a couple of flags. this patch returns the flag 'noparse', and that seems to fix
the problem.

attachment 5678.patch ignored as obsolete

ssanbeg wrote:

The patch returns the 'noargs' flag, not noparse (which wouldn't do what you want).

Currently, it seems that you can pass arbitrary arguments into the parser
functions, which would substitue into the result. i.e. changing the first
example to

{{ #if: {{{2|}}} | The articles {{{1}}} and {{{2}}} are nice. | The article
{{{1}}} is nice. |1={{{1}}}|2={{{2}}} }}

Should replace the clobbered arguments.

The patch would break some functionalty, i.e. {{#if:{{{1}}}|have {{{text}}}|no
{{{text}}} found|text=some long string...}} would stop working. But that
probably wasn't intentional, so it shouldn't be missed.

ayg wrote:

*** Bug 9054 has been marked as a duplicate of this bug. ***

updated patch

Have updated the patch to work with the current branch. it includes also fix
for CoreParserfunctions.php

a test page can be found, if my computer is online at
http://aza.nehle.net/~azatoth/srcwiki/index.php/5678

Attached:

bumping up the priority as people are getting more and more angry of the bug, and as it's pretty difficult to explain why it wont work as expected to ordinary editors, this bug should be prioritized.

This sort of bug, if extant, is a. worrying, b. eating loads of template developers time.

Can you create a parser test case for this to illustrate the problem and the fix? Probably adding a basic test case file for ParserFunctions would be a good start here.

Also, can you explain 'YAP'? :)

Fixed in the new parser (not live yet)

I'll close all the bugs the new parser fixes *when* it goes live. There are a lot.

We're moving closer to deploying this fix on Wikimedia now. We (especially Splarka, MZMcBride and me) have found quite a few templates that rely on this bug. No doubt when the fix goes live, some things will be broken.

For instance, incorrect use of {{!}}. Due to this bug, {{!}} can be used to separate template parameters, if the template call is inside a parser function.

For instance, where [[Template:Passthru]] contains {{{1}}}:

No template call in either parser:
{{passthru {{!}} foo}} -> {{passthru | foo}}

Template call due to bug 5678:
{{#if: 1 | {{passthru {{!}} foo}} }} -> foo

Behaviour in new parser:
{{#if: 1 | {{passthru {{!}} foo}} }} -> {{passthru | foo}}

http://en.wikipedia.org/w/index.php?title=Template:Infobox_Settlement&diff=184162381&oldid=183551988

We have also seen a case where a pipe was passed through to a template in the middle of an argument, and then that template used a parser function and bug 5678 to split the argument at the pipe. Then the resulting two arguments were passed through to a second template.

http://en.wikipedia.org/w/index.php?title=Image:PBB_GE_TCEA1_216241_s_at_fs.png&oldid=178652665&action=edit

Such syntax will need to be migrated.

  • Bug 12488 has been marked as a duplicate of this bug. ***

webboy wrote:

*** Bug 12828 has been marked as a duplicate of this bug. ***

Mormegil set Security to None.