Page MenuHomePhabricator

Categories/default sort sometimes duplicated to random position in DOM after edit
Closed, ResolvedPublic

Description

Details

Reference
bz50120

Event Timeline

bzimport raised the priority of this task from to Unbreak Now!.Nov 22 2014, 1:59 AM
bzimport set Reference to bz50120.

Perhaps related to:

PLACE OF BIRTH =[Alexandria], [[Egypt]]

We believe that this is now fixed (due to fixes in Parsoid). Please re-open if it recurs.

Is this a Parsoid DSR issue? Content getting repeated in this way (especially, the substituted template) is hard to see occurring in VisualEditor.

This may be related to bug 50853 which gabriel is going to be investigating to day.

https://en.wikipedia.org/w/index.php?title=Mat_Devine&curid=28997812&diff=563130933&oldid=563130533 looks like a VE bug to me. I can't reproduce it any more (similar to the case in bug 50853), so I am guessing that something in last night's VE deploy fixed this.

Closing as fixed, please reopen if this still happens.

Note the additional table screw-up in that one. We may need some cross-browser hammering on those revs to see if it's a browser-specific issue.

Couple more:
https://en.wikipedia.org/w/index.php?title=Peter_Biddle&diff=563624872&oldid=563623995
https://en.wikipedia.org/w/index.php?title=Jayaprakash_Narayan&diff=563627722&oldid=563627392

Based on inspection of recent diffs, I'd call this the single most prevalent and most problematic content corruption issue at this point.

This may be a VE bug (unconfirmed). Here is what I did.

I parsed mw:Jayaprakash Narayan on my local parsoid install and saved the HTML.
I then added a <link rel="mw:WikiLink/Category" href="Category:foobar"> (a new category essentially mimicking editor behavior), but I added it between the empty span that marks the opening of the Persondata tmeplate and the <table> that is part of the template. This effectively splits the template and duplicates the rest of the template.

If you look at the diff in https://en.wikipedia.org/w/index.php?title=Jayaprakash_Narayan&diff=563627722&oldid=563627392, all the categories are between the end of the template and the table. The above experiment yielded something similar, except in the diff, all categories are moved up.

So, it does seem that when a user adds categories, new/old categories are being moved/inserted between the empty span and the table breaking the atomic encapsulated template into two.

Also note that this only seems to affect Persondata template

  • in original wikitext, default sort template immediately follows the persondata template.
  • it has an empty span before/after the table
  • it has display:none set on it which means it doesn't show up in the editor.

Not sure if cursor position affects where categories are inserted. Can VE folks verify this hypothesis?

And, I misspoke. The spans from the template before/after the table are not really "empty" -- they have whitespace. And, the more interesting thing is that these spans do not get the display:none; css style but the table gets it from the style on the table ==> the spans are technically visible (with whitespace ignored in the browser) in VE, but the table is not.

So, there may also be a Parsoid issue here about how such templates are parsed.

(In reply to comment #15)

So, there may also be a Parsoid issue here about how such templates are
parsed.

As long as the content (including ws-only spans) is properly encapsulated that should not be relevant for this corruption.

I have seen VE move categories to random places in the DOM before. Apparently that bug is still alive. And hard to reproduce, sadly.

That's really strange. The template is one unit in the VE data model, so if <link> tags are placed in the middle of it that must be a bug in the data model -> HTML conversion, not in the data model itself.

See also: bug 50554, bug 50385.

  • Bug 50554 has been marked as a duplicate of this bug. ***

Subbu, Gabriel and I figured this out on IRC, and Subbu and Gabriel are working on a fix. Summary for the benefit of those following this bug:

  • On the first parse (either upon first VE load after the cache is purged, or upon the first edit after the purge), Parsoid parses the PERSONDATA template from scratch (because there is no cached content to reuse) and does so correctly. The output is something like <span>\n</span><table>...</table><link>
  • On the second parse, (first or second edit after cache purge), Parsoid reuses the template expansion from the first parse. It notices that the first (span) and last (link) nodes are both inline, and so it assumes the entire template is inline and wraps it in a <p>
  • The browser receives this HTML and is unhappy about the <table> inside the <p>, so it moves both the <table> and the <link> out of the <p>, leaving <p><span>\n</span></p><table>...</table><link>. Because the table is not a sibling of the span, VE doesn't recognize the table (or the link) as part of the template. Due to a separate bug in VE, the newline after the link is moved and ends up between the table and the link.
  • VE sends this corrupted HTML back to Parsoid, which freaks out and duplicates the table as well as a bunch of categories.
  • After the page is edited again (possibly by the user saving the corrupted VE output, possibly some other way), the third parse occurs, and Parsoid again tries to reuse the previous parse's expansion of the template. However, because of the <p> interruption, it only sees the span and doesn't see the table or the link. The table and the link disappear from the output in this and all subsequent parses, masking the bug. The user doesn't notice because the table has style="display:none;"

Change 73113 had a related patch set uploaded by GWicke:
Bug 50120: Avoid paragraph wrapping for DOM fragments with blocks

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

Change 73113 merged by jenkins-bot:
Bug 50120: Avoid paragraph wrapping for DOM fragments with blocks

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

The Parsoid fix is deployed. The P-wrapping portion is verified fixed on our test case [[Tim_Gartrell]].

The VE newline migration should be tracked in a different bug. Closing this bug as fixed.