Page MenuHomePhabricator

Do not let the visual mode break tables
Open, LowPublic

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.

Event Timeline

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 added a subscriber: ssastry.

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.

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.

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?

JTannerWMF moved this task from To Triage to Freezer on the VisualEditor board.Apr 9 2019, 3:16 PM
JTannerWMF added a subscriber: JTannerWMF.

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

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.
(…)

(…)
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.