Page MenuHomePhabricator
Create Task
Maniphest T166801

Visual diff: Template replaced by another template is shown as an attribute change
Closed, ResolvedPublic8 Estimated Story Points
Actions

Description

If you go change one template to another, the only difference is attributes, even if it's a completely different template. We need a way for nodes to override the basic node comparison of aNode.type === bNode.type.

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
Compare template names when diffing, not just typemediawiki/extensions/VisualEditormaster+11 -0
Update VE core submodule to master (4d6745b2f)mediawiki/extensions/VisualEditormaster+1 -1
Abstract definition of type equality when comparing inline nodesVisualEditor/VisualEditormaster+75 -8
Update VE core submodule to master (b3a7707)mediawiki/extensions/VisualEditormaster+1 -1
VisualDiff: Fix doc child replacementVisualEditor/VisualEditormaster+23 -10
Customize query in gerrit

Related Objects

Event Timeline

Esanders created this task.Jun 1 2017, 4:33 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 1 2017, 4:33 PM
Jdforrester-WMF triaged this task as Medium priority.Jun 1 2017, 5:31 PM
Jdforrester-WMF moved this task from To Triage to TR6: Visual diffs on the VisualEditor board.
Jdforrester-WMF set the point value for this task to 8.
Esanders added a comment.Jun 1 2017, 9:11 PM

Actually the typeChange property is never used, so changing any block node to any other block node will come up as an attribute change.

Esanders claimed this task.Jun 2 2017, 4:00 PM
gerritbot subscribed.Jun 6 2017, 2:58 PM

Change 357397 had a related patch set uploaded (by Esanders; owner: Esanders):
[VisualEditor/VisualEditor@master] VisualDiff: Fix doc child replacement

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

gerritbot added a project: Patch-For-Review.Jun 6 2017, 2:58 PM
gerritbot added a comment.Jun 25 2017, 2:03 AM

Change 357397 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] VisualDiff: Fix doc child replacement

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

• Deskana closed this task as Resolved.Jun 26 2017, 9:55 AM
Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptJun 26 2017, 9:55 AM
gerritbot added a comment.Jun 27 2017, 3:15 PM

Change 361679 had a related patch set uploaded (by Jforrester; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (b3a7707)

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

gerritbot added a comment.Jun 27 2017, 3:22 PM

Change 361679 merged by Jforrester:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (b3a7707)

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

Jdforrester-WMF edited projects, added VisualEditor-VisualDiffs; removed Patch-For-Review.Jun 27 2017, 3:32 PM
Esanders reopened this task as Open.Jul 26 2017, 2:22 PM

The patch only resolved the more egregious case when the node type has changed - we didn't add the special rule for templates yet.

Esanders mentioned this in T171377: Changing a reference shows useless and confusing details in visual diff.Jul 26 2017, 2:23 PM
gerritbot added a comment.Jul 26 2017, 5:19 PM

Change 367921 had a related patch set uploaded (by Esanders; owner: Esanders):
[VisualEditor/VisualEditor@master] Abstract definition of type equality when comparing inline nodes

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

gerritbot added a project: Patch-For-Review.Jul 26 2017, 5:19 PM
gerritbot added a comment.Jul 26 2017, 5:21 PM

Change 367922 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/VisualEditor@master] Compare template names when diffing, not just type

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

gerritbot added a comment.Jul 31 2017, 7:04 PM

Change 367921 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] Abstract definition of type equality when comparing inline nodes

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

gerritbot added a comment.Jul 31 2017, 7:08 PM

Change 368830 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (4d6745b2f)

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

gerritbot added a comment.Jul 31 2017, 7:18 PM

Change 368830 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (4d6745b2f)

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

• Deskana subscribed.Jul 31 2017, 7:20 PM

It seems we're not quite done here whilst https://gerrit.wikimedia.org/r/#/c/367922/ remains unmerged.

gerritbot added a comment.Jul 31 2017, 7:28 PM

Change 367922 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Compare template names when diffing, not just type

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

• Deskana closed this task as Resolved.Jul 31 2017, 7:32 PM

Speak of the devil.

ReleaseTaggerBot added a project: MW-1.30-release-notes (WMF-deploy-2017-08-01_(1.30.0-wmf.12)).Jul 31 2017, 8:00 PM
Jdforrester-WMF removed a project: Patch-For-Review.Jul 31 2017, 8:50 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits