[MediaWiki-commits] [Gerrit] Fix broken node change detection - change (VisualEditor/VisualEditor)

[Jforrester (Code Review)]

Jforrester has uploaded a new change for review.

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

Change subject: Fix broken node change detection
......................................................................

Fix broken node change detection

Currently this.selectedNode is always undefined.

Change-Id: Ia60661e22bf33bec7b863cefe35a6262c1a90c11
(cherry picked from commit c750849334ab18a448aa1597fa6b3e1f4703f32f)
---
M src/dm/ve.dm.Surface.js
1 file changed, 14 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/VisualEditor/VisualEditor 
refs/changes/36/167336/1

diff --git a/src/dm/ve.dm.Surface.js b/src/dm/ve.dm.Surface.js
index 36bcd8a..185cea8 100644
--- a/src/dm/ve.dm.Surface.js
+++ b/src/dm/ve.dm.Surface.js
@@ -574,9 +574,7 @@
        var left, right, leftAnnotations, rightAnnotations, 
insertionAnnotations,
                startNode, selectedNode, range,
                branchNodes = {},
-               oldSelection = this.selection,
-               oldBranchNodes = this.branchNodes,
-               oldSelectedNode = this.selectedNode,
+               selectionChange = false,
                contextChange = false,
                linearData = this.getDocument().data;
 
@@ -591,10 +589,11 @@
                return;
        }
 
-       // Update state
-       this.selection = selection;
-       this.branchNodes = branchNodes;
-       this.selectedNode = selectedNode;
+       // this.selection needs to be updated before we call 
setInsertionAnnotations
+       if ( !this.selection.equals( selection ) ) {
+               selectionChange = true;
+               this.selection = selection;
+       }
 
        if ( selection instanceof ve.dm.LinearSelection ) {
                range = selection.getRange();
@@ -658,17 +657,20 @@
 
        // If branchNodes or selectedNode changed emit a contextChange
        if (
-               selectedNode !== oldSelectedNode ||
-               branchNodes.start !== oldBranchNodes.start ||
-               branchNodes.end !== oldBranchNodes.end
+               selectedNode !== this.selectedNode ||
+               branchNodes.start !== this.branchNodes.start ||
+               branchNodes.end !== this.branchNodes.end
        ) {
+               this.branchNodes = branchNodes;
+               this.selectedNode = selectedNode;
                contextChange = true;
        }
 
-       // Emit events
-       if ( !oldSelection.equals( this.selection ) ) {
+       // If selection changed emit a select
+       if ( selectionChange ) {
                this.emit( 'select', this.selection.clone() );
        }
+
        if ( contextChange ) {
                this.emitContextChange();
        }

-- 
To view, visit https://gerrit.wikimedia.org/r/167336
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia60661e22bf33bec7b863cefe35a6262c1a90c11
Gerrit-PatchSet: 1
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: wmf/1.25wmf4
Gerrit-Owner: Jforrester <jforres...@wikimedia.org>
Gerrit-Reviewer: Esanders <esand...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits