diff --git a/modules/ve/actions/ve.AnnotationAction.js b/modules/ve/actions/ve.AnnotationAction.js index d6dea7f525..a27e53f046 100644 --- a/modules/ve/actions/ve.AnnotationAction.js +++ b/modules/ve/actions/ve.AnnotationAction.js @@ -42,7 +42,7 @@ ve.AnnotationAction.static.methods = ['set', 'clear', 'toggle', 'clearAll']; * @param {Object} [data] Additional annotation data */ ve.AnnotationAction.prototype.set = function ( name, data ) { - this.surface.getModel().getFragment().annotateContent( 'set', name, data ); + this.surface.getModel().getFragment().annotateContent( 'set', name, data ).destroy(); }; /** @@ -53,7 +53,7 @@ ve.AnnotationAction.prototype.set = function ( name, data ) { * @param {Object} [data] Additional annotation data */ ve.AnnotationAction.prototype.clear = function ( name, data ) { - this.surface.getModel().getFragment().annotateContent( 'clear', name, data ); + this.surface.getModel().getFragment().annotateContent( 'clear', name, data ).destroy(); }; /** @@ -68,9 +68,11 @@ ve.AnnotationAction.prototype.clear = function ( name, data ) { */ ve.AnnotationAction.prototype.toggle = function ( name, data ) { var fragment = this.surface.getModel().getFragment(); - fragment.annotateContent( - fragment.getAnnotations().hasAnnotationWithName( name ) ? 'clear' : 'set', name, data - ); + fragment + .annotateContent( + fragment.getAnnotations().hasAnnotationWithName( name ) ? 'clear' : 'set', name, data + ) + .destroy(); }; /** @@ -92,6 +94,7 @@ ve.AnnotationAction.prototype.clearAll = function ( filter ) { for ( i = 0, len = arr.length; i < len; i++ ) { fragment.annotateContent( 'clear', arr[i].name, arr[i].data ); } + fragment.destroy(); }; /* Registration */ diff --git a/modules/ve/actions/ve.ContentAction.js b/modules/ve/actions/ve.ContentAction.js index 3aa2c5befa..8797646b8c 100644 --- a/modules/ve/actions/ve.ContentAction.js +++ b/modules/ve/actions/ve.ContentAction.js @@ -42,7 +42,7 @@ ve.ContentAction.static.methods = ['insert', 'remove', 'select']; * @param {boolean} annotate Content should be automatically annotated to match surrounding content */ ve.ContentAction.prototype.insert = function ( content, annotate ) { - this.surface.getModel().getFragment().insertContent( content, annotate ); + this.surface.getModel().getFragment().insertContent( content, annotate ).destroy(); }; /** @@ -51,7 +51,7 @@ ve.ContentAction.prototype.insert = function ( content, annotate ) { * @method */ ve.ContentAction.prototype.remove = function () { - this.surface.getModel().getFragment().removeContent(); + this.surface.getModel().getFragment().removeContent().destroy(); }; /** diff --git a/modules/ve/actions/ve.FormatAction.js b/modules/ve/actions/ve.FormatAction.js index 8bdf7b9f95..4c63461641 100644 --- a/modules/ve/actions/ve.FormatAction.js +++ b/modules/ve/actions/ve.FormatAction.js @@ -49,7 +49,7 @@ ve.FormatAction.prototype.convert = function ( type, attributes ) { surfaceView = this.surface.getView(), surfaceModel = this.surface.getModel(), selection = surfaceModel.getSelection(), - fragmentForSelection = new ve.dm.SurfaceFragment( surfaceModel, selection, true ), + fragmentForSelection = surfaceModel.getFragment( selection, true ), doc = surfaceModel.getDocument(), fragments = []; @@ -61,13 +61,14 @@ ve.FormatAction.prototype.convert = function ( type, attributes ) { selected[i].node.getParent() : selected[i].node; - fragments.push( new ve.dm.SurfaceFragment( surfaceModel, contentBranch.getOuterRange(), true ) ); + fragments.push( surfaceModel.getFragment( contentBranch.getOuterRange(), true ) ); } for ( i = 0, length = fragments.length; i < length; i++ ) { - fragments[i].isolateAndUnwrap( type ); + fragments[i].isolateAndUnwrap( type ).destroy(); } selection = fragmentForSelection.getRange(); + fragmentForSelection.destroy(); txs = ve.dm.Transaction.newFromContentBranchConversion( doc, selection, type, attributes ); surfaceModel.change( txs, selection ); diff --git a/modules/ve/actions/ve.IndentationAction.js b/modules/ve/actions/ve.IndentationAction.js index 2045d0c979..3f18609049 100644 --- a/modules/ve/actions/ve.IndentationAction.js +++ b/modules/ve/actions/ve.IndentationAction.js @@ -65,9 +65,10 @@ ve.IndentationAction.prototype.increase = function () { this.indentListItem( documentModel.getNodeFromOffset( fragments[i].getRange().start + 1 ) ); + fragments[i].destroy(); } - selected.select(); + selected.select().destroy(); return increased; }; @@ -103,9 +104,10 @@ ve.IndentationAction.prototype.decrease = function () { this.unindentListItem( documentModel.getNodeFromOffset( fragments[i].getRange().start + 1 ) ); + fragments[i].destroy(); } - selected.select(); + selected.select().destroy(); return decreased; }; @@ -299,6 +301,7 @@ ve.IndentationAction.prototype.unindentListItem = function ( listItem ) { ); surfaceModel.change( tx ); } + fragment.destroy(); }; /* Registration */ diff --git a/modules/ve/dm/ve.dm.SurfaceFragment.js b/modules/ve/dm/ve.dm.SurfaceFragment.js index b8ab47d53c..c2797804eb 100644 --- a/modules/ve/dm/ve.dm.SurfaceFragment.js +++ b/modules/ve/dm/ve.dm.SurfaceFragment.js @@ -29,9 +29,10 @@ ve.dm.SurfaceFragment = function VeDmSurfaceFragment( surface, range, noAutoSele this.surface = surface; this.document = surface.getDocument(); this.noAutoSelect = !!noAutoSelect; + this.onTransactHandler = ve.bind( this.onTransact, this ); // Events - surface.on( 'transact', ve.bind( this.onTransact, this ) ); + surface.on( 'transact', this.onTransactHandler ); // Initialization var length = this.document.data.getLength(); @@ -107,6 +108,21 @@ ve.dm.SurfaceFragment.prototype.isNull = function () { return this.surface === undefined; }; +/** + * Destroys fragment, removing event handlers and object references, leaving the fragment null. + * + * Call this whenever you are done using a fragment. + * + * @method + */ +ve.dm.SurfaceFragment.prototype.destroy = function () { + if ( this.surface ) { + this.surface.removeListener( 'transact', this.onTransactHandler ); + this.surface = null; + this.document = null; + } +}; + /** * Get a new fragment with an adjusted position * diff --git a/modules/ve/test/dm/ve.dm.SurfaceFragment.test.js b/modules/ve/test/dm/ve.dm.SurfaceFragment.test.js index b95de75fa8..092cbfb11e 100644 --- a/modules/ve/test/dm/ve.dm.SurfaceFragment.test.js +++ b/modules/ve/test/dm/ve.dm.SurfaceFragment.test.js @@ -24,6 +24,7 @@ QUnit.test( 'constructor', 8, function ( assert ) { assert.equal( fragment.getRange().from, 0, 'range is clamped between 0 and document length' ); assert.equal( fragment.getRange().to, 61, 'range is clamped between 0 and document length' ); assert.strictEqual( fragment.willAutoSelect(), false, 'noAutoSelect values are boolean' ); + fragment.destroy(); } ); QUnit.test( 'onTransact', 1, function ( assert ) { @@ -37,6 +38,9 @@ QUnit.test( 'onTransact', 1, function ( assert ) { new ve.Range( 1, 1 ), 'fragment ranges are auto-translated when transactions are processed' ); + + fragment1.destroy(); + fragment2.destroy(); } ); QUnit.test( 'adjustRange', 3, function ( assert ) { @@ -47,6 +51,7 @@ QUnit.test( 'adjustRange', 3, function ( assert ) { assert.ok( fragment !== adjustedFragment, 'adjustRange produces a new fragment' ); assert.deepEqual( fragment.getRange(), new ve.Range( 20, 21 ), 'old fragment is not changed' ); assert.deepEqual( adjustedFragment.getRange(), new ve.Range( 1, 56 ), 'new range is used' ); + fragment.destroy(); } ); QUnit.test( 'collapseRange', 3, function ( assert ) { @@ -57,17 +62,22 @@ QUnit.test( 'collapseRange', 3, function ( assert ) { assert.ok( fragment !== collapsedFragment, 'collapseRange produces a new fragment' ); assert.deepEqual( fragment.getRange(), new ve.Range( 20, 21 ), 'old fragment is not changed' ); assert.deepEqual( collapsedFragment.getRange(), new ve.Range( 20, 20 ), 'new range is used' ); + collapsedFragment.destroy(); + fragment.destroy(); } ); QUnit.test( 'expandRange (closest)', 1, function ( assert ) { var doc = ve.dm.example.createExampleDocument(), surface = new ve.dm.Surface( doc ), - fragment = new ve.dm.SurfaceFragment( surface, new ve.Range( 20, 21 ) ); + fragment = new ve.dm.SurfaceFragment( surface, new ve.Range( 20, 21 ) ), + exapandedFragment = fragment.expandRange( 'closest', 'invalid type' ); assert.strictEqual( - fragment.expandRange( 'closest', 'invalid type' ).isNull(), + exapandedFragment.isNull(), true, 'closest with invalid type results in null fragment' ); + exapandedFragment.destroy(); + fragment.destroy(); } ); QUnit.test( 'expandRange (word)', 1, function ( assert ) { @@ -101,6 +111,8 @@ QUnit.test( 'expandRange (word)', 1, function ( assert ) { word = cases[i].phrase.substring( range.start, range.end ); assert.strictEqual( word, cases[i].expected, cases[i].msg + ': text' ); assert.strictEqual( cases[i].range.isBackwards(), range.isBackwards(), cases[i].msg + ': range direction' ); + fragment.destroy(); + newFragment.destroy(); } } ); @@ -123,6 +135,7 @@ QUnit.test( 'removeContent', 2, function ( assert ) { new ve.Range( 1, 1 ), 'removing content results in a zero-length fragment' ); + fragment.destroy(); } ); QUnit.test( 'insertContent', 3, function ( assert ) { @@ -146,6 +159,7 @@ QUnit.test( 'insertContent', 3, function ( assert ) { ['3', '2', '1'], 'strings get converted into data when inserting content' ); + fragment.destroy(); } ); QUnit.test( 'wrapNodes/unwrapNodes', 10, function ( assert ) { @@ -191,6 +205,7 @@ QUnit.test( 'wrapNodes/unwrapNodes', 10, function ( assert ) { fragment.unwrapNodes( 0, 2 ); assert.deepEqual( doc.getData(), originalDoc.getData(), 'unwrapping 2 levels restores document to original state' ); assert.deepEqual( fragment.getRange(), new ve.Range( 55, 61 ), 'range after unwrapping is same as original range' ); + fragment.destroy(); // Make a 1 paragraph into 1 list with 1 item fragment = new ve.dm.SurfaceFragment( surface, new ve.Range( 9, 12 ) ); @@ -219,12 +234,13 @@ QUnit.test( 'wrapNodes/unwrapNodes', 10, function ( assert ) { fragment.unwrapNodes( 0, 2 ); assert.deepEqual( doc.getData(), originalDoc.getData(), 'unwrapping 2 levels restores document to original state' ); assert.deepEqual( fragment.getRange(), new ve.Range( 9, 12 ), 'range after unwrapping is same as original range' ); + fragment.destroy(); fragment = new ve.dm.SurfaceFragment( surface, new ve.Range( 8, 34 ) ); fragment.unwrapNodes( 3, 1 ); assert.deepEqual( fragment.getData(), doc.getData( new ve.Range( 5, 29 ) ), 'unwrapping multiple outer nodes and an inner node' ); assert.deepEqual( fragment.getRange(), new ve.Range( 5, 29 ), 'new range contains inner elements' ); - + fragment.destroy(); } ); QUnit.test( 'rewrapNodes', 4, function ( assert ) { @@ -268,6 +284,7 @@ QUnit.test( 'rewrapNodes', 4, function ( assert ) { // The intermediate stage (plain text attached to the document) would be invalid // if performed as an unwrap and a wrap expectedData = ve.copyArray( doc.getData() ); + fragment.destroy(); fragment = new ve.dm.SurfaceFragment( surface, new ve.Range( 59, 65 ) ); fragment.rewrapNodes( 1, [ { 'type': 'heading', 'attributes': { 'level': 1 } } ] ); @@ -279,6 +296,7 @@ QUnit.test( 'rewrapNodes', 4, function ( assert ) { assert.deepEqual( doc.getData(), expectedData, 'rewrapping paragraphs as headings' ); assert.deepEqual( fragment.getRange(), new ve.Range( 59, 65 ), 'new range contains rewrapping elements' ); + fragment.destroy(); } ); QUnit.test( 'wrapAllNodes', 10, function ( assert ) { @@ -317,6 +335,7 @@ QUnit.test( 'wrapAllNodes', 10, function ( assert ) { fragment.unwrapNodes( 0, 2 ); assert.deepEqual( doc.getData(), originalDoc.getData(), 'unwrapping 2 levels restores document to original state' ); assert.deepEqual( fragment.getRange(), new ve.Range( 55, 61 ), 'range after unwrapping is same as original range' ); + fragment.destroy(); // Make a 1 paragraph into 1 list with 1 item fragment = new ve.dm.SurfaceFragment( surface, new ve.Range( 9, 12 ) ); @@ -345,6 +364,7 @@ QUnit.test( 'wrapAllNodes', 10, function ( assert ) { fragment.unwrapNodes( 0, 2 ); assert.deepEqual( doc.getData(), originalDoc.getData(), 'unwrapping 2 levels restores document to original state' ); assert.deepEqual( fragment.getRange(), new ve.Range( 9, 12 ), 'range after unwrapping is same as original range' ); + fragment.destroy(); fragment = new ve.dm.SurfaceFragment( surface, new ve.Range( 5, 37 ) ); @@ -360,6 +380,7 @@ QUnit.test( 'wrapAllNodes', 10, function ( assert ) { expectedData, 'unwrapping 4 levels (table, tableSection, tableRow and tableCell)' ); + fragment.destroy(); } ); QUnit.test( 'rewrapAllNodes', 6, function ( assert ) { @@ -389,6 +410,7 @@ QUnit.test( 'rewrapAllNodes', 6, function ( assert ) { 'rewrapping multiple nodes via a valid intermediate state produces the same document as unwrapping then wrapping' ); assert.deepEqual( fragment.getRange(), expectedFragment.getRange(), 'new range contains rewrapping elements' ); + expectedFragment.destroy(); // Reverse of first test fragment.rewrapAllNodes( @@ -412,6 +434,7 @@ QUnit.test( 'rewrapAllNodes', 6, function ( assert ) { 'rewrapping multiple nodes via a valid intermediate state produces the same document as unwrapping then wrapping' ); assert.deepEqual( fragment.getRange(), new ve.Range( 5, 37 ), 'new range contains rewrapping elements' ); + fragment.destroy(); // Rewrap a heading as a paragraph // The intermediate stage (plain text attached to the document) would be invalid @@ -424,6 +447,7 @@ QUnit.test( 'rewrapAllNodes', 6, function ( assert ) { assert.deepEqual( doc.getData(), expectedData, 'rewrapping a heading as a paragraph' ); assert.deepEqual( fragment.getRange(), new ve.Range( 0, 5 ), 'new range contains rewrapping elements' ); + fragment.destroy(); } ); function runIsolateTest( assert, type, range, expected, label ) { @@ -437,6 +461,7 @@ function runIsolateTest( assert, type, range, expected, label ) { expected( data ); assert.deepEqual( doc.getFullData(), data, label ); + fragment.destroy(); } QUnit.test( 'isolateAndUnwrap', 4, function ( assert ) { diff --git a/modules/ve/ui/inspectors/ve.ui.LinkInspector.js b/modules/ve/ui/inspectors/ve.ui.LinkInspector.js index 713cb95fea..73bb34a2d8 100644 --- a/modules/ve/ui/inspectors/ve.ui.LinkInspector.js +++ b/modules/ve/ui/inspectors/ve.ui.LinkInspector.js @@ -69,7 +69,8 @@ ve.ui.LinkInspector.prototype.initialize = function () { * @method */ ve.ui.LinkInspector.prototype.onSetup = function () { - var fragment = this.surface.getModel().getFragment( null, true ), + var expandedFragment, trimmedFragment, truncatedFragment, + fragment = this.surface.getModel().getFragment( null, true ), annotation = this.getMatchingAnnotations( fragment ).get( 0 ); // Call parent method @@ -79,24 +80,33 @@ ve.ui.LinkInspector.prototype.onSetup = function () { if ( !annotation ) { if ( fragment.getRange().isCollapsed() ) { // Expand to nearest word - fragment = fragment.expandRange( 'word' ); + expandedFragment = fragment.expandRange( 'word' ); + fragment.destroy(); + fragment = expandedFragment; } else { // Trim whitespace - fragment = fragment.trimRange(); + trimmedFragment = fragment.trimRange(); + fragment.destroy(); + fragment = trimmedFragment; } if ( !fragment.getRange().isCollapsed() ) { // Create annotation from selection - annotation = this.getAnnotationFromTarget( fragment.truncateRange( 255 ).getText() ); + truncatedFragment = fragment.truncateRange( 255 ); + fragment.destroy(); + fragment = truncatedFragment; + annotation = this.getAnnotationFromTarget( fragment.getText() ); fragment.annotateContent( 'set', annotation ); this.isNewAnnotation = true; } } else { // Expand range to cover annotation - fragment = fragment.expandRange( 'annotation', annotation ); + expandedFragment = fragment.expandRange( 'annotation', annotation ); + fragment.destroy(); + fragment = expandedFragment; } // Update selection - fragment.select(); + fragment.select().destroy(); }; /** @@ -118,6 +128,8 @@ ve.ui.LinkInspector.prototype.onOpen = function () { this.targetInput.setAnnotation( annotation ); this.targetInput.$input.focus().select(); }, this ), 200 ); + + fragment.destroy(); }; /** @@ -130,7 +142,7 @@ ve.ui.LinkInspector.prototype.onClose = function ( action ) { // Call parent method ve.ui.Inspector.prototype.onClose.call( this, action ); - var i, len, annotations, selection, + var i, len, annotations, selection, adjustedFragment, insert = false, undo = false, clear = false, @@ -160,7 +172,10 @@ ve.ui.LinkInspector.prototype.onClose = function ( action ) { } if ( insert ) { // Insert default text and select it - fragment = fragment.insertContent( target, false ).adjustRange( -target.length, 0 ); + fragment.insertContent( target, false ); + adjustedFragment = fragment.adjustRange( -target.length, 0 ); + fragment.destroy(); + fragment = adjustedFragment; // Move cursor to the end of the inserted content selection = new ve.Range( this.initialSelection.start + target.length ); @@ -189,6 +204,8 @@ ve.ui.LinkInspector.prototype.onClose = function ( action ) { ); // Reset state this.isNewAnnotation = false; + + fragment.destroy(); }; /** diff --git a/modules/ve/ui/ve.ui.Context.js b/modules/ve/ui/ve.ui.Context.js index 83380f5226..500929dc08 100644 --- a/modules/ve/ui/ve.ui.Context.js +++ b/modules/ve/ui/ve.ui.Context.js @@ -165,7 +165,7 @@ ve.ui.Context.prototype.destroy = function () { */ ve.ui.Context.prototype.update = function () { var i, nodes, items, - fragment = this.surface.getModel().getFragment(), + fragment = this.surface.getModel().getFragment( null, false ), selection = fragment.getRange(), inspector = this.inspectors.getCurrent(); @@ -194,6 +194,9 @@ ve.ui.Context.prototype.update = function () { if ( items.length ) { // There's at least one inspectable annotation, build a menu and show it this.$menu.empty(); + if ( this.toolbar ) { + this.toolbar.destroy(); + } this.toolbar = new ve.ui.Toolbar( $( '
' ), this.surface, @@ -210,6 +213,8 @@ ve.ui.Context.prototype.update = function () { // Remember selection for next time this.selection = selection.clone(); + fragment.destroy(); + return this; }; diff --git a/modules/ve/ui/ve.ui.Toolbar.js b/modules/ve/ui/ve.ui.Toolbar.js index 3d44309b16..6390e3965d 100644 --- a/modules/ve/ui/ve.ui.Toolbar.js +++ b/modules/ve/ui/ve.ui.Toolbar.js @@ -24,9 +24,10 @@ ve.ui.Toolbar = function VeUiToolbar( $container, surface, config ) { this.$ = $container; this.$groups = $( '
' ); this.config = config || {}; + this.onContextChangeHandler = ve.bind( this.onContextChange, this ); // Events - this.surface.getModel().on( 'contextChange', ve.bind( this.onContextChange, this ) ); + this.surface.getModel().on( 'contextChange', this.onContextChangeHandler ); // Initialization this.$groups.addClass( 've-ui-toolbarGroups' ); @@ -68,7 +69,7 @@ ve.ui.Toolbar.prototype.getSurface = function () { */ ve.ui.Toolbar.prototype.onContextChange = function () { var i, len, leafNodes, - fragment = this.surface.getModel().getFragment(), + fragment = this.surface.getModel().getFragment( null, false ), nodes = []; leafNodes = fragment.getLeafNodes(); @@ -78,6 +79,7 @@ ve.ui.Toolbar.prototype.onContextChange = function () { } } this.emit( 'updateState', nodes, fragment.getAnnotations(), fragment.getAnnotations( true ) ); + fragment.destroy(); }; /** @@ -107,3 +109,15 @@ ve.ui.Toolbar.prototype.setup = function () { this.$groups.append( $group ); } }; + +/** + * Destroys toolbar, removing event handlers and DOM elements. + * + * Call this whenever you are done using a toolbar. + * + * @method + */ +ve.ui.Toolbar.prototype.destroy = function () { + this.surface.getModel().removeListener( 'contextChange', this.onContextChangeHandler ); + this.$.remove(); +}; diff --git a/modules/ve/ve.Surface.js b/modules/ve/ve.Surface.js index b645132a17..fe05cedece 100644 --- a/modules/ve/ve.Surface.js +++ b/modules/ve/ve.Surface.js @@ -170,7 +170,7 @@ ve.Surface.prototype.isEnabled = function () { * @method */ ve.Surface.prototype.resetSelection = function () { - this.model.getFragment().select(); + this.model.getFragment().select().destroy(); this.view.surfaceObserver.poll(); };