From 51fe6913ee8c24245f31209478176a3eec729859 Mon Sep 17 00:00:00 2001
From: Isaiah Odhner <isaiahodhner@gmail.com>
Date: Sun, 15 Dec 2019 20:54:03 -0500
Subject: [PATCH] Fix parenting of image transform undoables

`cancel()` inside `undoable()` action callback caused it to navigate history,
and it would navigate all the way to the start of the last pointer gesture because
`history_node_to_cancel_to` was only set on pointerdown, not taking into account non-pointer-gesture operations.
The point was to allow undoing Fill With Color when canceling, but with the old definition of `history_node_to_cancel_to` it would have needed to be set in scattered places.
Now I'm defining it where it can be null, and if it's null, canceling should just go back to the history node at the start of `cancel()` (after making history nodes if applicable by telling tools to finish).
---
 src/app.js                |  8 ++++++--
 src/functions.js          | 16 ++++++++++++----
 src/image-manipulation.js |  4 ++--
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/src/app.js b/src/app.js
index 45892c3..027ed7e 100644
--- a/src/app.js
+++ b/src/app.js
@@ -58,7 +58,7 @@ let text_tool_font = {
 
 let root_history_node = make_history_node({name: "App Not Loaded Properly - Please send a bug report."}); // will be replaced
 let current_history_node = root_history_node;
-let history_node_to_cancel_to = current_history_node;
+let history_node_to_cancel_to = null;
 /** array of history nodes */
 let undos = [];
 /** array of history nodes */
@@ -734,7 +734,7 @@ $canvas.on("pointerdown", e => {
 
 		$G.on("pointermove", canvas_pointer_move);
 
-		$G.one("pointerup", (e) => {
+		$G.one("pointerup", (e, canceling) => {
 			button = undefined;
 			reverse = false;
 
@@ -752,6 +752,10 @@ $canvas.on("pointerdown", e => {
 			for (const interval_id of interval_ids) {
 				clearInterval(interval_id);
 			}
+
+			if (!canceling) {
+				history_node_to_cancel_to = null;
+			}
 		});
 	};
 
diff --git a/src/functions.js b/src/functions.js
index 90c31d5..82850a8 100644
--- a/src/functions.js
+++ b/src/functions.js
@@ -267,7 +267,8 @@ function reset_file(){
 function reset_canvas_and_history(){
 	undos.length = 0;
 	redos.length = 0;
-	history_node_to_cancel_to = current_history_node = root_history_node = make_history_node({name: "New Document"});
+	current_history_node = root_history_node = make_history_node({name: "New Document"});
+	history_node_to_cancel_to = null;
 
 	canvas.width = Math.max(1, my_canvas_width);
 	canvas.height = Math.max(1, my_canvas_height);
@@ -1044,7 +1045,12 @@ function undoable({name, icon, use_loose_canvas_changes, soft}, callback){
 
 	saved = false;
 
+	const before_callback_history_node = current_history_node;
 	callback && callback();
+	if (current_history_node !== before_callback_history_node) {
+		show_error_message(`History node switched during undoable callback for ${name}. This shouldn't happen.`);
+		window.console && console.log(`History node switched during undoable callback for ${name}, from`, before_callback_history_node, "to", current_history_node);
+	}
 
 	const image_data = ctx.getImageData(0, 0, canvas.width, canvas.height);
 
@@ -1230,7 +1236,8 @@ function show_document_history() {
 function cancel(going_to_history_node){
 	// Note: this function should be idempotent.
 	// `cancel(); cancel();` should do the same thing as `cancel();`
-	$G.triggerHandler("pointerup");
+	history_node_to_cancel_to = history_node_to_cancel_to || current_history_node;
+	$G.triggerHandler("pointerup", ["canceling"]);
 	for (const selected_tool of selected_tools) {
 		selected_tool.cancel && selected_tool.cancel();
 	}
@@ -1239,6 +1246,7 @@ function cancel(going_to_history_node){
 		// which isn't good, but there's no real conflict resolution in multi-user mode anyways
 		go_to_history_node(history_node_to_cancel_to, true);
 	}
+	history_node_to_cancel_to = null;
 	update_helper_layer();
 }
 function meld_selection_into_canvas(going_to_history_node) {
@@ -1448,9 +1456,9 @@ function image_invert(){
 }
 
 function clear(){
+	deselect();
+	cancel();
 	undoable({name: "Clear"}, () => {
-		deselect();
-		cancel();
 		saved = false;
 
 		if(transparency){
diff --git a/src/image-manipulation.js b/src/image-manipulation.js
index 73a9567..5dfb249 100644
--- a/src/image-manipulation.js
+++ b/src/image-manipulation.js
@@ -413,12 +413,12 @@ function apply_image_transformation(meta, fn){
 			selection.replace_source_canvas(new_canvas);
 		});
 	}else{
+		deselect();
+		cancel();
 		undoable({
 			name: meta.name,
 			icon: meta.icon,
 		}, () => {
-			deselect();
-			cancel();
 			saved = false;
 			
 			ctx.copy(new_canvas);