Last modified: 2014-09-04 00:20:01 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T52120, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 50120 - Categories/default sort sometimes duplicated to random position in DOM after edit
Categories/default sort sometimes duplicated to random position in DOM after ...
Status: RESOLVED FIXED
Product: VisualEditor
Classification: Unclassified
Data Model (Other open bugs)
unspecified
All All
: Highest major
: VE-deploy-2013-07-11
Assigned To: Ed Sanders
:
: 50554 (view as bug list)
Depends on:
Blocks: 50848
  Show dependency treegraph
 
Reported: 2013-06-24 18:55 UTC by MZMcBride
Modified: 2014-09-04 00:20 UTC (History)
8 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description MZMcBride 2013-06-24 18:55:38 UTC
https://en.wikipedia.org/w/index.php?title=Dina_bint_%27Abdu%27l-Hamid&diff=561398396&oldid=561397974

I added a single letter ("therafter" --> "thereafter"). VisualEditor duplicated the categories and the default sort.
Comment 1 MZMcBride 2013-06-24 18:57:06 UTC
Perhaps related to:

| PLACE OF BIRTH    =[Alexandria], [[Egypt]]
Comment 3 James Forrester 2013-06-24 21:23:50 UTC
Roan, thoughts?
Comment 5 James Forrester 2013-07-05 19:42:23 UTC
We believe that this is now fixed (due to fixes in Parsoid). Please re-open if it recurs.
Comment 6 Erik Moeller 2013-07-06 17:06:53 UTC
This is still occurring. Most recent example from a few minutes ago:

https://en.wikipedia.org/w/index.php?title=Mat_Devine&curid=28997812&diff=563130933&oldid=563130533
Comment 7 James Forrester 2013-07-08 16:30:12 UTC
Is this a Parsoid DSR issue? Content getting repeated in this way (especially, the substituted template) is hard to see occurring in VisualEditor.
Comment 8 ssastry 2013-07-08 17:16:17 UTC
This may be related to bug 50853 which gabriel is going to be investigating to day.
Comment 9 Gabriel Wicke 2013-07-09 19:59:33 UTC
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.
Comment 12 Erik Moeller 2013-07-10 07:04:50 UTC
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.
Comment 13 Erik Moeller 2013-07-10 07:22:23 UTC
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.
Comment 14 ssastry 2013-07-10 16:52:32 UTC
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?
Comment 15 ssastry 2013-07-10 16:58:28 UTC
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.
Comment 16 Gabriel Wicke 2013-07-10 17:07:18 UTC
Possibly related: bug 50332.
Comment 17 Gabriel Wicke 2013-07-10 17:14:26 UTC
(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.
Comment 18 Roan Kattouw 2013-07-10 18:04:09 UTC
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.
Comment 19 Gabriel Wicke 2013-07-10 19:12:55 UTC
Another case:
https://en.wikipedia.org/w/index.php?title=Frederick_Calvert,_6th_Baron_Baltimore&curid=884173&diff=563705563&oldid=563705411

Also updated the subject and moved to VE.
Comment 20 Gabriel Wicke 2013-07-10 19:20:37 UTC
See also: bug 50554, bug 50385.
Comment 21 Gabriel Wicke 2013-07-10 19:23:28 UTC
*** Bug 50554 has been marked as a duplicate of this bug. ***
Comment 23 Roan Kattouw 2013-07-10 22:15:53 UTC
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;"
Comment 24 Gerrit Notification Bot 2013-07-10 22:19:10 UTC
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
Comment 25 Gerrit Notification Bot 2013-07-10 23:12:10 UTC
Change 73113 merged by jenkins-bot:
Bug 50120: Avoid paragraph wrapping for DOM fragments with blocks

https://gerrit.wikimedia.org/r/73113
Comment 26 Gabriel Wicke 2013-07-10 23:51:25 UTC
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.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links