Section edit links disappear following an unclosed {{
Open, MediumPublic

Description

This was seen in revisions prior to https://en.wikipedia.org/w/index.php?title=Wikipedia:VisualEditor/Feedback&diff=561926823&oldid=561926781 and fixed in that revision.

For a reduced testcase see https://test.wikipedia.org/wiki/Section_edit_links_bug2. The cause seems to be the unclosed {{. Of course, this is broken wikitext, but the parser seems to be able to handle it fine (other than leaving out section edit links).

Sorry I can't be more specific.


Version: 1.22.0
Severity: normal
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=73182

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 1:46 AM
bzimport added a project: MediaWiki-Parser.
bzimport set Reference to bz50369.
bzimport added a subscriber: Unknown Object (MLST).
matmarex renamed this task from Section edit links disappear in certain circumstances to Section edit links disappear following an unclosed {{.Feb 3 2022, 10:22 PM
matmarex updated the task description. (Show Details)
matmarex added subscribers: Umherirrender, IKhitron, mfb and 5 others.
matmarex subscribed.

Does this still occur if you add ?useparsoid=1 to the URL?

Parsoid has a different bug in this scenario: the section edit links for the affected sections are shown, but they link to the wrong sections (the numbering seems to restart at 1 after the unmatched {{).

I had this problem at https://en.wikipedia.org/w/index.php?title=Wikipedia_talk:WikiProject_Templates&oldid=1305235543 when trying to edit the last section.

If you copy that wikitext to a sandbox page on en.WP and then try to edit the last section, it will likely give an error. I fixed it by closing unclosed braces in another section. The existing parser lets me edit the last section just fine. Parsoid should either be tolerant of this syntax error or render a giant error message on the page so that someone will fix it instead of trying to keep editing as normal.

Change #507966 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/services/parsoid@master] Use wikipeg rule variable to ensure headingIndex is correct after backtrack

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

Change #507966 merged by jenkins-bot:

[mediawiki/services/parsoid@master] Use wikipeg rule variable to ensure headingIndex is correct after backtrack

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

Change #1187841 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.22.0-a21

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

Change #1187841 merged by jenkins-bot:

[mediawiki/vendor@master] Bump wikimedia/parsoid to 0.22.0-a21

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

I don't see any changes to Parsoid's behavior in this case after these patches.

I don't see any changes to Parsoid's behavior in this case after these patches.

0.22.0-a21 isn't deployed yet though

I tested locally, as well as https://en.wikipedia.beta.wmcloud.org/w/index.php?title=Section_edit_links_bug2&useparsoid=1, which should have the new version, right?

Ya, not fixed. I downloaded that wikitext locally and tried it as well.

Ok, a minimal reproducer is:

== Foo ==
== Bar ==
<code>{{</code>
== Bat ==
}}
== Baz ==

This is actually subtly different from T222419: Incorrect section numbering after unclosed subst. What is happening here is that we're "parsing" a complete template between the braces: {{</code>\n== Bat ==\n}}:

0-[peg]        | ---->    [{"type":"TagTk","name":"code","attribs":[],"dataParsoid":{"tsr":[20,26],"stx":"html"}},{"type":"SelfclosingTagTk","name":"template","attribs":[{"k":[{"type":"EndTagTk","name":"code","attribs":[],"dataParsoid":{"tsr":[28,35],"stx":"html"}},{"type":"NlTk","dataParsoid":{"tsr":[35,36]}},"== Bat ==",{"type":"NlTk","dataParsoid":{"tsr":[45,46]}}],"v":"","srcOffsets":[28,46,46,46]}],"dataParsoid":{"tsr":[26,48],"src":"{{</code>\n== Bat ==\n}}"}}]

and the heading isn't given a section id here because it's not recognized as a heading inside the template arguments (maybe a bug). Then we later realize that </code> is not a valid template name and dump the tokens back to string form.

We don't match the heading in the template target position because we're matching inlineline_in_tpls?. The template3 grammar patches would fix this by immediately jumping to the preproc grammar once we see a {{, I'll work on those.

Change #1243368 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/services/parsoid@master] WIP: parser test cases for T52369

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

This issue has become more urgent with the rollout of Parsoid to discussion pages. I have run into it a few times in the past week, and it's a topic on WP:VPT on en.WP. See https://en.wikipedia.org/w/index.php?title=Wikipedia:Village_pump_(technical)&oldid=1365462662#Why_have_section_edit_previews_changed? which is about two different Parsoid rendering and editing issues.

Here is a simple test showing and describing what happens with and without parsoid when there are 5 sections, and {{ in section 2 is paired with }} in section 4: https://en.wikipedia.org/wiki/User_talk:PrimeHunter/Parsoid_section_edit_error

On the surface, PrimeHunter's bug looks different ("Parsoid shows bad section edit links where they should be absent") from what this bug title effectively says ("Parsoid doesn't show section edit links after unclosed {{"). But, I suspect the bug behavior has diverged after all the fixes to T391624 and Parsoid's handing is manifesting as this new buggy behavior (which is a bit more serious since it leads to the wrong sections being edited and potentially saved).

According to https://en.wikipedia.org/wiki/Wikipedia:Village_pump_(technical)/Parsoid#Timeline Parsoid was rolled out to Talk namespaces on 20 July, which is probably why this neglected bug is popping up with some frequency in the last couple of days. Some attention to this bug, or reverting the rollout of Parsoid to Talk namespaces, may be helpful.

For Parsoid devs: So, what is happening is that the improperly tokenized template token is converted back to a string and that string is retokenized in a new pipeline whose heading index starts at 0 and is assigned new headings. So, two things needed to fix this. (a) template retokenization should suppress heading assignment (b) heading index in the main grammar should be bumped up by the # of headings encountered in the retokenized string. (a) is easy but (b) is slightly trickier given that we removed headingIndex as a tokenizer state variable in Scott's patch above ( T52369#11170098 ). So, this might be a TokenStreamPatcher fixup potentially.

Nothing new to add that you don't already know, just a kind of anecdotal 'user impact statement' to add to Jonesey's comment:

As a senior editor used to wikicode editing, when I want to create a new Talk page discussion, I always use the edit-section link on the last available section on the page, and start my new section under it, so that I can provide a complete edit summary of my choice rather than use the Start-new-section link or tab, which generally don't permit that.

My most recent attempt to start a section ran into this bug, and manifested as an inability to add a new discussion to the user Talk page, because attempting to edit the last section failed. My workaround for now is to edit the entire page (with its concomitant disadvantages of load time, risk of edit conflict, etc.) so it negatively impacts my ability to raise any new TP discussion. (After experiencing the problem, I ran over to VPT to add my two cents, and as it happens, I still haven't gotten around to adding that new TP discussion I was planning on.)

My first test https://en.wikipedia.org/wiki/User_talk:PrimeHunter/Parsoid_section_edit_error had an opening {{ in section 2 and a closing }} in section 4. This caused odd behaviour, especially with Parsoid.

My second test https://en.wikipedia.org/wiki/User_talk:PrimeHunter/Parsoid_section_edit_error_with_template_syntax_never_closed also has an opening {{ in section 2, but never closes it. All sections have a normal correctly numbered section edit link which goes to the section, both with and without Parsoid, so there is no longer any error.

It appears there is only a problem if an open {{ in one section happens to be matched by a closing }} in a later section.

Change #1314320 had a related patch set uploaded (by Subramanya Sastry; author: Subramanya Sastry):

[mediawiki/services/parsoid@master] WIP: Fixup heading indexes in some edge cases

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

> My second test https://en.wikipedia.org/wiki/User_talk:PrimeHunter/Parsoid_section_edit_error_with_template_syntax_never_closed also has an opening {{ in section 2, but never closes it. All sections have a normal correctly numbered section edit link which goes to the section, both with and without Parsoid, so there is no longer any error.

Yes, Scott's patch above ( T52369#11170098 tooke care of this scenario.

My WIP patch above takes care of the {{ .. }} case. With any luck, we should have it deployed next week.

While we are working on the patch for the {{..}} scenarios, @ABreault-WMF was asking me whether using nowikis to escape the "{{" might work for now. I thought that was an interesting idea and I figured I would ask you editors who responded here to chime in on whether using <nowiki>{{</nowiki> would be workable as a workaround? That would fix Parsoid handling and more importantly, also bring you edit section links on the intervening sections (which currently are missing them). Ex: Compare https://en.wikipedia.org/w/index.php?title=User:SSastry_(WMF)/T52369&useparsoid=0 vs https://www.mediawiki.org/w/index.php?title=User:SSastry_(WMF)/Sandbox&useparsoid=0

We'll still work through the patch and improving handling of these constructs, but it seems like nowiki-escaping problematic constructs is worth considering anyway.

All stray "{{" and "}}" should always be nowikied or removed, also if they aren't currently causing a problem. It's a problem waiting to happen even when there are no bugs.

There is some understandable unhappiness around whether we should add more complexity to the codebase to handle this. We may still solve some version of this if it falls out cleanly and generally improves the codebase. So, if editors are happy with that resolution around nowiking {{ (for this bug just nowiking the opening pair is sufficient to address it), that might be a better overall solution instead of us trying to add more code to handle this.

https://gerrit.wikimedia.org/r/1314320 is a complete working patch to solve this very generally, but we are discussing if we should do it a different way instead, but which might still leave some edge cases lying around, and also if we really need to do this.

nowiki is wikitext so I assumed you asked whether editors should add nowiki if they spot a page with stray "{{" and "}}" causing the issue. I meant that editors should always add nowiki or remove even if there is no current issue or only one of them is present. I have done that for many years. The problem is that an editor has to spot it and know what to do so it's not a realistic way to fix most instances of this bug.,

The problem is that an editor has to spot it and know what to do so it's not a realistic way to fix most instances of this bug.,

That is a good and fair observation. Would a lint category targeting this help?

The problem is that an editor has to spot it and know what to do so it's not a realistic way to fix most instances of this bug.,

Could an edit filter spot it, at least in a section edit? That would be a win, as then it would never get saved in the first place. Pinging a couple of edit filter managers whose name I recognized on the list: @Anachronist, @GorillaWarfare, @HouseBlaster.