Page MenuHomePhabricator

Empty section causes later section to not be parsed correctly
Closed, ResolvedPublic

Description

In a Form with multiple sections, if the one is left empty but a later one populated, then attempting to edit the page later using the form will result in the whole later section appearing inside the text field of an earlier populated section.

Partial Form definition:

{{{end template}}}

'''Story:'''

{{{section|Story|hide if empty}}}

'''Gameplay:'''

{{{section|Gameplay|hide if empty}}}

'''Screenshots:'''

{{{section|Screenshots|hide if empty}}}

'''External Links:'''

{{{section|External Links|hide if empty}}}

'''Free text:'''

Screenshot of "Gameplay" section having gobbled the contents and heading of the "External Links" section:

Screenshot from 2020-02-27 13-01-37.png (625×290 px, 24 KB)

Event Timeline

Hi @Sparr, thanks for taking the time to report this and welcome to Wikimedia Phabricator!

Can you please include exact MediaWiki version information, and the exact version of PageForms used? Thanks.

MediaWiki 1.34.0 (408a7ce)
Page Forms 4.7-alpha (7865d64)

@Sparr I have uploaded a solution https://gerrit.wikimedia.org/r/c/mediawiki/extensions/PageForms/+/588150. I don't know why it isn't being displayed here. Please give it a try and tell me if it works.

@Sahajsk: It's not displayed here because you did not follow the Commit message guidelines. :) Remove the empty line after Bug: T246380

If I make the change now will it show?

Change 588150 had a related patch set uploaded (by Sahajsk; owner: Sahajsk):
[mediawiki/extensions/PageForms@master] Make empty section to parse later sections correctly

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

@Yaron_Koren I can't figure out why it isn't working for you even if we are doing the same things, so I have attached my form and page definitions here.
Form definition:

<noinclude>
This is the "Gadgets" form.
To create a page with this form, enter the page name below;
if a page with that name already exists, you will be sent to a form to edit that page.


{{#forminput:form=Gadgets}}

</noinclude><includeonly>
<div id="wikiPreview" style="display: none; padding-bottom: 25px; margin-bottom: 25px; border-bottom: 1px solid #AAAAAA;"></div>
{{{for template|Gadgets}}}
{| class="formtable"
! Gadget: 
| {{{field|gadget}}}
|-
! Date: 
| {{{field|date|input type=date}}}
|-
! Datetime: 
| {{{field|datetime|input type=datetime}}}
|}
{{{end template}}}

'''Story:'''

{{{section|Story|hide if empty}}}

'''Gameplay:'''

{{{section|Gameplay|hide if empty}}}

'''Screenshots:'''

{{{section|Screenshots|hide if empty}}}

'''External Links:'''

{{{section|External Links|hide if empty}}}

'''Free text:'''

{{{standard input|free text|rows=10}}}


{{{standard input|summary}}}

{{{standard input|minor edit}}} {{{standard input|watch}}}

{{{standard input|save}}} {{{standard input|preview}}} {{{standard input|changes}}} {{{standard input|cancel}}}
</includeonly>

Page definition:

{{Gadgets
|gadget=tv
|date=2062/03/31
|datetime=2002/03/21 12:00:00 AM
}}
==Gameplay==
game

==Screenshots==
screen

Page created using form:

image.png (570×646 px, 30 KB)

Editing this page with the same form:
image.png (787×930 px, 27 KB)

Thank you for trying this out.

Okay - it wasn't working for when I tried it with a form that only contained sections, however, with a form that has a template and then sections, this patch does indeed fix things.

Change 588150 merged by jenkins-bot:
[mediawiki/extensions/PageForms@master] Make empty section to parse later sections correctly

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

Yaron_Koren claimed this task.

I believe this can now be marked as resolved!

Reopening this bug. @Sahajsk - did you see this?

Sure I'll look for the issue and improvise the code.

I guess the issue is different section names in form definition and the page.
Form: The community/communities behind this submission.
Page: The communities behind this submission.

Making them same makes the code work fine.
This didn't happen in previous versions as the earlier code used to skip this section in the page.
@Nikerabbit can please you try that.

@Sahajsk - that's a good find (assuming @Nikerabbit agrees that this is indeed the problem), but a form should never get into an infinite loop, if at all possible, no matter what is on the page. It doesn't really matter what shows up in the form, with this kind of bad input - I just want to make sure that something gets displayed. Can you fix the infinite loop problem?

Yes, I am already onto it.
Presently I have implemented a workaround which displays a warning with the section name at the top and displays the form below( Sections might not be parsed correctly but showed as before this change).
Another option is to display just the error message and no form like

image.png (172×461 px, 9 KB)

Can you please suggest me a warning message as per Mediawiki convention?

I don't think a warning of any kind is necessary... Page Forms generally doesn't display warning messages for things like that; it just tries to do the best that it can with the current page text.

Change 595522 had a related patch set uploaded (by Sahajsk; owner: Sahajsk):
[mediawiki/extensions/PageForms@master] Update empty section parsing to avoid infinite loop Additional check has been made to exit the loop if none of the conditions is met.

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

Change 595522 merged by jenkins-bot:
[mediawiki/extensions/PageForms@master] Update empty section parsing to avoid infinite loop

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

@Yaron_Koren: Hi, the patch in Gerrit has been merged. Can this task be resolved (via Add Action...Change Status in the dropdown menu), or is there more to do in this task? Also, do you have any idea if this maybe also fixed T246088? Thanks in advance! :)

Well, I was waiting to hear back from @Nikerabbit , but I suppose this can be closed regardless - done.

I don't know if this also fixed T246088.

Oh I see, sorry! :) Nikerabbit: Please reopen if this did not help.

Sorry, I was too busy listening to your podcast :). I didn't test this issue (yet), but I've tested the other ones.

Well, that's a good excuse! :) Hopefully everything is resolved now.