Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix logic bugs related to e1e9427 #1398

Merged
merged 1 commit into from
Oct 27, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion core/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,15 @@ Blockly.Events.filter = function(queueIn, forward) {
lastEvent.element == 'warningOpen')) {
// Merge click events.
lastEvent.newValue = event.newValue;
} else {
// Collision: newer events should merge into this event to maintain order
hash[key] = event;
mergedQueue.push(event);
}
}
}
queue = mergedQueue;
// Filter out any events that have become null due to merging.
queue = mergedQueue.filter(function(e) { return !e.isNull(); });
if (!forward) {
// Restore undo order.
queue.reverse();
Expand Down
48 changes: 48 additions & 0 deletions tests/jsunit/event_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,48 @@ function test_events_mergeUi() {
eventTest_tearDownWithMockBlocks();
}

/**
* Tests that events that collide on a (event, block, workspace) tuple
* but cannot be merged do not get dropped during filtering.
*/
function test_events_stackclick() {
eventTest_setUpWithMockBlocks();
var block = workspace.newBlock('field_variable_test_block', '1');
var events = [
new Blockly.Events.Ui(block, 'click', undefined, undefined),
new Blockly.Events.Ui(block, 'stackclick', undefined, undefined)
];
var filteredEvents = Blockly.Events.filter(events, true);
// click and stackclick should both exist
assertEquals(2, filteredEvents.length);
assertEquals('click', filteredEvents[0].element);
assertEquals('stackclick', filteredEvents[1].element);
eventTest_tearDownWithMockBlocks();
}

/**
* Mutator composition could result in move events for blocks
* connected to the mutated block that were null operations. This
* leads to events in the undo/redo queue that do nothing, requiring
* an extra undo/redo to proceed to the next event. This test ensures
* that two move events that do get merged (disconnecting and
* reconnecting a block in response to a mutator change) are filtered
* from the queue.
*/
function test_events_filteraftermerge() {
eventTest_setUpWithMockBlocks();
var block = workspace.newBlock('field_variable_test_block', '1');
block.setParent(null);
var events = [];
helper_addMoveEventParent(events, block, null);
helper_addMoveEventParent(events, block, null);
var filteredEvents = Blockly.Events.filter(events, true);
// The two events should be merged, but because nothing has changed
// they will be filtered out.
assertEquals(0, filteredEvents.length);
eventTest_tearDownWithMockBlocks();
}

/**
* Helper function to simulate block move events.
*
Expand All @@ -525,3 +567,9 @@ function helper_addMoveEvent(events, block, newX, newY) {
block.xy_ = new goog.math.Coordinate(newX, newY);
events[events.length-1].recordNew();
}

function helper_addMoveEventParent(events, block, parent) {
events.push(new Blockly.Events.BlockMove(block));
block.setParent(parent);
events[events.length-1].recordNew();
}