Page MenuHomePhabricator
Create Task
Maniphest T220319

Do not let the visual mode break tables
Open, LowPublic
Actions

Description

This is ultimately about this edit: https://en.wikipedia.org/w/index.php?title=MicroMasters&diff=prev&oldid=884550598&diffmode=source which was reported by User:Snipergang at https://www.mediawiki.org/wiki/Topic:Uxcjdi6r1iufk81k

How to break a table:

  1. Start with broken wikitext code for a table. It needs to leave a stray |- above the table in visual mode.
  2. Open the page in the visual editor.
  3. Delete the stray |- from the top of the page.
  4. Save the page.
  5. That changed the opening line of the table to <br />{| class="wikitable mw-collapsible", which is a disaster.
  6. Try something else: Delete the stray }- _and_ the blank line.
  7. Save the page.
  8. That put the opening code for the at the end of the previous paragraph, which is also a disaster.
  9. Report bug until it's no longer a disaster.

What we need: No matter what I do, don't let the opening code for the table markup be in any location other than the start of its own line.

Related Objects

Event Timeline

• Whatamidoing-WMF created this task.Apr 8 2019, 2:02 AM
Restricted Application added a project: VisualEditor. · View Herald TranscriptApr 8 2019, 2:02 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
ssastry triaged this task as Low priority.Apr 8 2019, 3:01 PM
ssastry subscribed.

The problematic edit that started these problems is this one which introduced a HTML table row which eventually led to these series of issues. Not sure what caused HTML to be introduced there. My guess is that it was a copy-paste that caused it to serialize as HTML.

But, once a table has broken wikitext, it gets harder and harder for subsequent parses and edits to fix the table. Ideally, we would introduce a pre-save linting mode to flag errors before save or introduce a post-save linting mode that flags these issues early so they can be fixed. That would be a different feature request.

So, I am going to mark this ticket itself low priority - if there is a reasonable simple fix for specific scenarios encountered here, we'll implement them once we are done with our current porting project, but, as I noted above, once we have a broken table, there will always be some scenarios where editing behavior will no longer look reasonable. So, it becomes a case of a whack-a-mole at that point.

ssastry added a comment.Apr 8 2019, 3:10 PM

https://en.wikipedia.org/w/index.php?title=User:SSastry_(WMF)/VE_Test/Sandbox&oldid=891530736 has source that can be used to reproduce the problem. Note that the HTML <td> cell needs to have a wikilink in it for the table to parsed in the way where the |- gets fostered outside the table. Without that, it parses quite differently. So, this is even more of an edge case than we thought.

ssastry added a comment.Apr 8 2019, 3:16 PM
In T220319#5094041, @ssastry wrote:

https://en.wikipedia.org/w/index.php?title=User:SSastry_(WMF)/VE_Test/Sandbox&oldid=891530736 has source that can be used to reproduce the problem. Note that the HTML <td> cell needs to have a wikilink in it for the table to parsed in the way where the |- gets fostered outside the table. Without that, it parses quite differently. So, this is even more of an edge case than we thought.

So, in this revision, given this markup |B<tr><td>P</td><td>Q</td></tr>|- .. the B<tr><td>P</td><td>Q</td></tr> between the pipes gets parsed as table-cell attributes and discarded since they are invalid attributes. But changing the <td>Q</td> to <td>[[Q]]</td>, changes the parse of that markup to something different and introduce this problem.

Recording this info now for when we revisit this issue at a later time. Anyway, overall, this is a very specific edge case. But, if at all possible now, it would be interesting to figure out how the original problem got introduced into the table as noted in T220319#5093998 ... but it may be somewhat late to know what happened in that edit session, but maybe VE devs have some idea?

ssastry edited projects, added Parsoid-Edit-Support; removed Parsoid.Apr 8 2019, 3:22 PM
JTannerWMF moved this task from To Triage to Freezer on the VisualEditor board.Apr 9 2019, 3:16 PM
JTannerWMF subscribed.

The Editing Team isn't planning to prioritize this work at this time, due to this appearing as an edge case.

matmarex subscribed.Apr 9 2019, 4:32 PM
In T220319#5093998, @ssastry wrote:

The problematic edit that started these problems is this one which introduced a HTML table row which eventually led to these series of issues. Not sure what caused HTML to be introduced there. My guess is that it was a copy-paste that caused it to serialize as HTML.

(…)

In T220319#5094080, @ssastry wrote:

(…)

Recording this info now for when we revisit this issue at a later time. Anyway, overall, this is a very specific edge case. But, if at all possible now, it would be interesting to figure out how the original problem got introduced into the table as noted in T220319#5093998 ... but it may be somewhat late to know what happened in that edit session, but maybe VE devs have some idea?

I remember a similar issue in the past caused by incorrectly copying a data-parsoid='{"stx":"html"}' attribute (or a RESTBase id hiding it) between unrelated nodes: T108506#4549272.

We fixed that one, but there may be other cases where we incorrectly copy data-parsoid/id attributes. Another one that I'm aware of is described on T203112.

Jonesey95 subscribed.Feb 2 2020, 4:35 AM

The following Visual Editor edit broke a correctly formatted table, as far as I know:

https://en.wikipedia.org/w/index.php?title=Shooting_Stars_Award&diff=next&oldid=938178150

In the edit, the end-of-table marker was removed. Visual Editor should not allow that sort of thing to happen.

I don't know if this is a separate bug or an instance of this bug.

Aklapper edited projects, added Parsoid; removed Parsoid-Edit-Support.Feb 29 2020, 5:15 PM
Arlolra moved this task from Needs Triage to Backlog on the Parsoid board.Mar 2 2020, 3:41 PM
ssastry moved this task from Backlog to Bugs & Crashers on the Parsoid board.May 14 2020, 10:36 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL