Page MenuHomePhabricator

VisualEditor moves main content unexpectedly up to another reuse when details are added (unintended diff)
Closed, ResolvedPublicBUG 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

Details

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

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.

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

[mediawiki/extensions/Cite@master] VE: Improve getSubRefs test setup

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

Change #1214018 merged by jenkins-bot:

[mediawiki/extensions/Cite@master] VE: Improve getSubRefs test setup

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

Change #1152657 merged by jenkins-bot:

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

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

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

[mediawiki/extensions/Cite@master] Update QUnit tests in preparation for code refactoring

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

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

[mediawiki/extensions/Cite@master] Update visualEditorHtml2WtTests

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

Change #1215108 merged by jenkins-bot:

[mediawiki/extensions/Cite@master] Update QUnit tests in preparation for code refactoring

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

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

[mediawiki/extensions/Cite@master] VE: Updating some Converter tests to reflect current Parsoid

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

Change #1215150 merged by jenkins-bot:

[mediawiki/extensions/Cite@master] VE: Updating some Converter tests to reflect current Parsoid

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

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

[mediawiki/extensions/Cite@master] VE: Use int for isSubRefWithMainBody

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

Change #1216543 merged by jenkins-bot:

[mediawiki/extensions/Cite@master] VE: Use int for isSubRefWithMainBody

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

Change #1191672 merged by jenkins-bot:

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

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

Change #1215109 merged by jenkins-bot:

[mediawiki/extensions/Cite@master] Update visualEditorHtml2WtTests

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

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

[mediawiki/extensions/Citoid@master] Update call to ve.dm.MWReferenceModel.insertReferenceNode

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

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

[mediawiki/extensions/Cite@master] Remove back-compatibility from to ve.dm.MWReferenceModel

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

Change #1255803 merged by jenkins-bot:

[mediawiki/extensions/Citoid@master] Update call to ve.dm.MWReferenceModel.insertReferenceNode

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

Change #1255806 merged by jenkins-bot:

[mediawiki/extensions/Cite@master] Remove back-compatibility from to ve.dm.MWReferenceModel

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