Page MenuHomePhabricator

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

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.

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 Normal 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.

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

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

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

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

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

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

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.

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

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

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

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

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

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

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

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

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.