Page MenuHomePhabricator

VisualEditor moves main content unexpectedly up to another reuse when details are added (unintended diff)
Open, Needs TriagePublicBUG REPORT

Description

Steps to replicate the issue (include links if applicable):

  • Assume the following Wikitext
<ref name="miller" />

Some content here

<ref name="miller">Miller: main content is part of 2nd occurence in text</ref>
  • Open VE
  • Convert both refs into sub-refs by adding details
  • Save the page

What happens?:

  • The resulting Wikitext looks like this and the main content moved up
<ref details="page 1" name="miller">Miller: main content is part of 2nd occurence in text</ref>

Some content here

<ref details="page 2" name="miller" />

What should have happened instead?:

  • The resulting Wikitext looks like this and the main content stays at the same place
<ref details="page 1" name="miller" />

Some content here

<ref details="page 2" name="miller">Miller: main content is part of 2nd occurence in text</ref>
Notes:
  • There is no content loss but this behavior might be considered as a dirty diff by some wikitext editors
  • Unintended diff
Breadcrumbs for the fix:
  • We currently just link the first sub-ref to the synthetic main content if needed this should be improved:
  • The 2nd main-ref's RefNode should have a contentsUsed attribute set
  • Make sure that this is also on the sub-ref's RefNode when converting to a sub-ref
  • Respect the contentsUsed attribute deciding on which sub-ref gets linked to the synthetic main ref

Event Timeline

Change #1188806 had a related patch set uploaded (by WMDE-Fisch; author: WMDE-Fisch):

[mediawiki/extensions/Cite@master] VE: Preserve the contentsUsed flag when adding details

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

Change #1189392 had a related patch set uploaded (by WMDE-Fisch; author: WMDE-Fisch):

[mediawiki/extensions/Cite@master] VE: Code cleanup in sub-ref creation

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

Change #1189392 merged by jenkins-bot:

[mediawiki/extensions/Cite@master] VE: Code cleanup in sub-ref creation

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

Change #1190234 had a related patch set uploaded (by WMDE-Fisch; author: WMDE-Fisch):

[mediawiki/extensions/Cite@master] VE: Renome method that checks if body content is already stored

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

Change #1190251 had a related patch set uploaded (by WMDE-Fisch; author: WMDE-Fisch):

[mediawiki/extensions/Cite@master] VE: Use the contentsUsed flag to match sub-refs that store main content

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

Change #1190234 abandoned by WMDE-Fisch:

[mediawiki/extensions/Cite@master] VE: Rename method that checks if body content is already stored

Reason:

Not renaming for now. I might refactor this later.

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

Change #1190234 restored by Thiemo Kreuz (WMDE):

[mediawiki/extensions/Cite@master] VE: Rename method that checks if body content is already stored

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

Change #1190234 merged by jenkins-bot:

[mediawiki/extensions/Cite@master] More readable code and comments in ve.dm.MWReferenceNode

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

Change #1190251 merged by jenkins-bot:

[mediawiki/extensions/Cite@master] VE: Use the contentsUsed flag to match sub-refs that store main content

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

Change #1188806 merged by jenkins-bot:

[mediawiki/extensions/Cite@master] VE: Preserve contentsUsed flag when converting main refs

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

Change #1191440 had a related patch set uploaded (by WMDE-Fisch; author: WMDE-Fisch):

[mediawiki/extensions/Cite@master] VE Converter: Move sub-ref check out on body content check

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

Change #1191672 had a related patch set uploaded (by WMDE-Fisch; author: WMDE-Fisch):

[mediawiki/extensions/Cite@master] VE Converter: Merge code that places the main content

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

Change #1191440 merged by jenkins-bot:

[mediawiki/extensions/Cite@master] VE Converter: Move sub-ref check out on body content check

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

Change #1152657 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/Cite@master] Make MWReferenceModel.insertReferenceNode more flexible

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

Change #1195975 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/Cite@master] Minor comment and code cleanups in ve.dm.MWReferenceNode

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

Change #1195976 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/Cite@master] Minor updates to test input

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

Change #1195975 merged by jenkins-bot:

[mediawiki/extensions/Cite@master] Minor comment and code cleanups in ve.dm.MWReferenceNode

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

Change #1195976 merged by jenkins-bot:

[mediawiki/extensions/Cite@master] Minor updates to test input

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

Verified as still broken, it looks like the remaining open patches are important.