Fix spurious MetaList events

Moving items around in the MetaList only works if all deletions are
done before all insertions. This was almost but not completely true:
when moving an existing item we would delete it and immediately re-
insert it, meaning that if there were multiple items being moved
there would be a removal after an insertion.

Instead, let the insertion loop re-insert the moved items while
making sure not to emit insert events for them. This is how I
originally intended the code to be structured, but I thought that
duplicating the insertion would be equivalent to protecting the
emission of the insertion event. Clearly I was wrong.

This fixes weird bugs where categories sometimes appeared twice
in the category dialog depending on where the category was in
the list and what the exact length of the list was (i.e., whether
the binary search in the MetaList found the first or second item
with identical offset and index).

Change-Id: I4cd131052dda396add7a0e2dbe2383bb5c1b5721
This commit is contained in:
Roan Kattouw 2013-11-27 09:29:15 -08:00 committed by Jforrester
parent bbba628f3c
commit 39eea83698

View file

@ -240,7 +240,7 @@ ve.dm.MetaList.prototype.onTransact = function ( tx ) {
if ( newItems[i].item.isAttached() ) {
if ( newItems[i].offset !== newItems[i].item.offset || newItems[i].index !== newItems[i].item.index ) {
this.deleteRemovedItem( newItems[i].item.offset, newItems[i].item.index );
this.addInsertedItem( newItems[i].offset, newItems[i].index, newItems[i].item );
newItems[i].preExisting = true;
}
}
}
@ -249,9 +249,11 @@ ve.dm.MetaList.prototype.onTransact = function ( tx ) {
for ( i = 0, ilen = newItems.length; i < ilen; i++ ) {
if ( !newItems[i].item.isAttached() ) {
this.addInsertedItem( newItems[i].offset, newItems[i].index, newItems[i].item );
if ( !newItems[i].preExisting ) {
events.push( [ 'insert', newItems[i].item ] );
}
}
}
// Emit events
for ( i = 0, ilen = events.length; i < ilen; i++ ) {