Page MenuHomePhabricator

Inserting image into table in VisualEditor creates a line break in the table wikicode
Closed, ResolvedPublic1 Story Points

Description

When I insert image via insert > media, I get unwanted line breaks;

see eg this my contribution on Czech Wikipedia:

https://cs.wikipedia.org/w/index.php?title=Seznam_pam%C3%A1tn%C3%BDch_strom%C5%AF_v_okrese_Chomutov&type=revision&diff=14244659&oldid=14207931

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 28 2016, 3:39 PM
Esanders added a subscriber: Esanders.

Actually making any change will reformat the table. This is a Parsoid issue.

Actually making any change will reformat the table. This is a Parsoid issue.

I tried 3 different edits on the latest version of the page and they behaved as expected -- no table reformatting or dirty diffs.

So, not yet sure what happened there.

Okay, so, here is another attempt at reproduction.

I edited https://cs.wikipedia.org/w/index.php?title=Seznam_pam%C3%A1tn%C3%BDch_strom%C5%AF_v_okrese_Chomutov&action=edit&oldid=14207931 inside VE, make a tiny change somewhere, and sure enough, I can reproduce the problem.

So, I try to replicate this locally.

  1. I downloaded the wikitext for this revision (14207931) and the HTML and data-parsoid from RESTBase
  2. I edited the data-parsoid file to add the {"parsoid": .. } wrapper to make it work with the parse.js script which expects the {"parsoid": ... } wrapper outside corresponding to data-parsoid.
  3. I copy the html to new.html, and edit the file manually to add a small change in a table cell.
  4. I then run this script parse.js --html2wt --selser --pbinfile /tmp/dp.json --oldhtmlfile /tmp/old.html --oldtextfile /tmp/old.wt < /tmp/new.html > /tmp/new.wt
  5. I look at the diff for old.wt and new.wt and the diffs are for just the change I made. I try this with a couple different edits, and same result each time.

So, it looks like VE might be somehow mangling the HTML during edit causing this? Given that it is not the case for the latest revision, is there something in the HTML output from Parsoid that is tripping the code? @Esanders, do you want to investigate this on the VE side?

https://gerrit.wikimedia.org/r/#/c/318659/ makes it easy to test edits on specific revisions just in case you want to reproduce my test.

Jdforrester-WMF triaged this task as Low priority.Nov 8 2016, 8:19 PM
Jdforrester-WMF moved this task from To Triage to TR0: Interrupt on the VisualEditor board.
Jdforrester-WMF set the point value for this task to 1.
This comment was removed by Esanders.

The issue is <p> tags inside <td>

Parsoid gives us table cells without <p> tags, but VE always wraps content in <p> tags. The non empty cells gets "wrapper" paragraphs which get removed when we convert back to HTML, the empty cells get "empty" paragraphs which only get removed if they stay empty, so we end up sending back <td><p>New content</p></td> which Parsoid inserts a linebreak after.

I think this is a Parsoid issue as <p> inside <td> shouldn't necessarily mean serialize with a newline.

The issue is <p> tags inside <td>
Parsoid gives us table cells without <p> tags, but VE always wraps content in <p> tags. The non empty cells gets "wrapper" paragraphs which get removed when we convert back to HTML, the empty cells get "empty" paragraphs which only get removed if they stay empty, so we end up sending back <td><p>New content</p></td> which Parsoid inserts a linebreak after.

But, the problem is that *unedited* content is seeing diffs and I cannot reproduce that outside VE. And, with the newest revision, I didn't see this problem. That is the reason I asked if there is something about the output of that revision (14207931) that is causing this.

Thanks for investigating this Esanders.

It seems to be something that happens with all tables in VE, no matter which type of content is added to a cell.

The issue is <p> tags inside <td>
...
I think this is a Parsoid issue as <p> inside <td> shouldn't necessarily mean serialize with a newline.

This piece is covered by T149209 which I'll look at this week.

The other part of the bug report (about dirty diffs elsewhere) could be a transient issue in VE with the p-tags not getting removed properly on save (just on that particular revision?) since I cannot reproduce the error locally.

Deskana closed this task as Resolved.Aug 30 2018, 10:09 AM
Deskana claimed this task.
Deskana added a subscriber: Deskana.

I can no longer reproduce this problem: I edited that same table and added an image to a cell, and as you can see in the diff no line break was inserted in the wikitext. I am assuming something happened in the intervening two years that fixed this.

Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptAug 30 2018, 10:09 AM