jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/393436 )
Change subject: Destroy CE nodes on removal by onSplice
......................................................................
Destroy CE nodes on removal by onSplice
Bug: T179574
Change-Id: Ic858711456bb786f0c1c9d6cfc4e26e11be283ac
---
M src/ce/nodes/ve.ce.ImageNode.js
M src/ce/ve.ce.BranchNode.js
M tests/ce/ve.ce.BranchNode.test.js
3 files changed, 17 insertions(+), 6 deletions(-)
Approvals:
DLynch: Looks good to me, approved
jenkins-bot: Verified
Jforrester: Looks good to me, but someone else must approve
diff --git a/src/ce/nodes/ve.ce.ImageNode.js b/src/ce/nodes/ve.ce.ImageNode.js
index 0d86851..2d083de 100644
--- a/src/ce/nodes/ve.ce.ImageNode.js
+++ b/src/ce/nodes/ve.ce.ImageNode.js
@@ -85,6 +85,11 @@
* @param {jQuery.Event} e Load event
*/
ve.ce.ImageNode.prototype.onLoad = function () {
+ if ( !this.model ) {
+ // This node has probably been destroyed. (Currently there's no
easy way for
+ // a mixin class to disconnect listeners on destroy)
+ return;
+ }
this.setOriginalDimensions( {
width: this.$image.prop( 'naturalWidth' ),
height: this.$image.prop( 'naturalHeight' )
diff --git a/src/ce/ve.ce.BranchNode.js b/src/ce/ve.ce.BranchNode.js
index b1a3131..82adcf0 100644
--- a/src/ce/ve.ce.BranchNode.js
+++ b/src/ce/ve.ce.BranchNode.js
@@ -203,6 +203,8 @@
removals[ i ].setLive( false );
removals[ i ].detach();
removals[ i ].$element.detach();
+ // And fare thee weel a while
+ removals[ i ].destroy();
}
if ( args.length >= 3 ) {
if ( index > 0 ) {
diff --git a/tests/ce/ve.ce.BranchNode.test.js
b/tests/ce/ve.ce.BranchNode.test.js
index cf2bd36..f9ca5ec 100644
--- a/tests/ce/ve.ce.BranchNode.test.js
+++ b/tests/ce/ve.ce.BranchNode.test.js
@@ -69,7 +69,8 @@
} );
QUnit.test( 'onSplice', function ( assert ) {
- var modelA = new ve.dm.BranchNodeStub(),
+ var viewB, viewC,
+ modelA = new ve.dm.BranchNodeStub(),
modelB = new ve.dm.BranchNodeStub(),
modelC = new ve.dm.BranchNodeStub(),
viewA = new ve.ce.BranchNodeStub( modelA );
@@ -78,18 +79,21 @@
modelA.splice( 0, 0, modelB, modelC );
assert.strictEqual( viewA.getChildren().length, 2 );
- assert.deepEqual( viewA.getChildren()[ 0 ].getModel(), modelB );
- assert.deepEqual( viewA.getChildren()[ 1 ].getModel(), modelC );
+ viewB = viewA.getChildren()[ 0 ];
+ viewC = viewA.getChildren()[ 1 ];
+ assert.deepEqual( viewB.getModel(), modelB, 'First view child matches
model tree' );
+ assert.deepEqual( viewC.getModel(), modelC, 'Second view child matches
model tree' );
// Removal tests
modelA.splice( 0, 1 );
-
assert.strictEqual( viewA.getChildren().length, 1 );
- assert.deepEqual( viewA.getChildren()[ 0 ].getModel(), modelC );
+ assert.deepEqual( viewA.getChildren()[ 0 ].getModel(), modelC, 'Only
view child matches model tree' );
+ assert.strictEqual( !viewB.getModel(), true, 'Removed view node was
destroyed' );
// Removal and insertion tests
modelA.splice( 0, 1, modelB );
assert.strictEqual( viewA.getChildren().length, 1 );
- assert.deepEqual( viewA.getChildren()[ 0 ].getModel(), modelB );
+ assert.deepEqual( viewA.getChildren()[ 0 ].getModel(), modelB,
'Replaced view child matches model tree' );
+ assert.strictEqual( !viewC.getModel(), true, 'Replaced view node was
destroyed' );
} );
--
To view, visit https://gerrit.wikimedia.org/r/393436
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic858711456bb786f0c1c9d6cfc4e26e11be283ac
Gerrit-PatchSet: 3
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Divec <[email protected]>
Gerrit-Reviewer: DLynch <[email protected]>
Gerrit-Reviewer: Divec <[email protected]>
Gerrit-Reviewer: Esanders <[email protected]>
Gerrit-Reviewer: Jforrester <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
