Page MenuHomePhabricator

The autoedit modify other instances of a multi-instance template form
Open, Needs TriagePublic

Description

With current version of PageForms (3f4a5866) with MediaWiki 1.37.0-alpha (ec0be22116) (but also reproductible with PF 5.1 + MW 1.33), when an autoedit should modify the last instance (or adds an instance) of a multi-instance template, it sometimes modify the first instance of the said template. See also relevant doc of this feature.

The exact caracterization could be improved, but here is a reproductible example.

PFtest:

{{PFtemplate
|type=sky
|colorSky=blue
}}
{{PFtemplate
|type=water
|colorWater=azul
}}
{{PFtemplate
|type=wood
|defaultColor=brown
}}

Template:PFtemplate:

The {{{type|…}}} is {{#switch:{{{type|}}}
|sky = {{{colorSky|{{{defaultColor|…}}}}}}, which is beautiful
|water = {{{colorWater|{{{defaultColor|…}}}}}}, which is nice
|wood = {{{colorWood|{{{defaultColor|…}}}}}}, which is dark
|#default = {{{defaultColor|…}}}
}}.

Form:PFform:

{{{for template|PFtemplate|multiple|add button text=Add a template}}}
* Type : {{{field|type|input type=text}}}
* Color (sky) : {{{field|colorSky|input type=text}}}
* Color (water) : {{{field|colorWater|input type=text}}}
* Color (wood) : {{{field|colorWood|input type=text}}}
* Color (default) : {{{field|defaultColor|input type=text}}}
{{{end template}}}
----
{{{standard input|save}}}
{{{standard input|cancel}}}

PFautoedit:

{{#autoedit:form=PFform|target=PFtest|link text=Modify the wood|query string=PFtemplate[2][colorWood]=dark brown}}

When you click on the autoedit button, you obtain the following diff:

@@ -2,4 +2,5 @@
 |type=sky
 |colorSky=blue
+|colorWood=dark brown
 }}
 {{PFtemplate
@@ -9,4 +10,5 @@
 {{PFtemplate
 |type=wood
+|colorWood=dark brown
 |defaultColor=brown
 }}

It should instead be the following diff since the autoedit is instructed to modify the third instance of the template:

@@ -9,4 +9,5 @@
 {{PFtemplate
 |type=wood
+|colorWood=dark brown
 |defaultColor=brown
 }}

PS: azul is blue in Portuguese, one of the sole words I know in this language thanks to Cesaria Evora in the song Mar azul :)

Event Timeline

Current state of my investivations: in includes/PF_AutoeditAPI.php in doAction(), the variables $preloadContent contains the real original content of the target page, but, in the first $wgPageFormsFormPrinter->formHTML (inside the if ( $preloadContent !== '' ) { around lines 906-915), the $targetContent contains a modification of the first instance of the template (which is not intended) and consequently the $formHTML contains both modification (the real one at the end, and the wrong one at the beginning).

Tracking the bug with a debugger, it seems the issues is around $tif->setFieldValuesFromSubmit(); in PFFormPrinter::formHTML(): it seems to be assumed that the number of instances provided by the submitted request (N) corresponds to the number of modified instances (from 0 to N-1). This is correct when the entire form is submitted (classic case) but incorrect in the autoedit case: you can modify only 1 instance of the template and it could be another than the first.

Change 685540 had a related patch set uploaded (by Seb35; author: Seb35):

[mediawiki/extensions/PageForms@master] Modify only some instances of a multi-instance template

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

@Seb35 I've tried to reproduce the bug with the same template & form you've mentioned. But I believe the code works fine now. Can you confirm whether you're using the latest version of PageForms ?

Seb35 updated the task description. (Show Details)

@Techwizzie I just retried with the lastest version of PageForms (7ea66e9c6 and cherry-picking patchset 5 from Gerrit (commit 84c192e61b4)) and it is still the same behaviour (buggy except if I misunderstood the documentation): the autoedit should modify the third instance of the template PFtemplate, but it actually modify the first and the third instances of PFtemplate.

I added in the description a second diff, which should be the result according to my understanding of this feature.