From 0d4846d012e1bf877497b4f4d6071078069838cd Mon Sep 17 00:00:00 2001
From: Thomas Goyne <plorkyeran@aegisub.org>
Date: Fri, 6 Apr 2012 01:55:14 +0000
Subject: [PATCH] Handle the cancel button in the color picker dialog better

Return wxNullColor rather than the original color on cancel so that the
calling code can actually tell if it was cancelled.

If one of the subs edit box color buttons is cancelled, undo the changes
made rather than restoring the original text of the active line. This
makes the cancel button actually work with multiple lines selected, and
eliminates some undo state noise.

Closes #1465.

Originally committed to SVN as r6663.
---
 aegisub/src/colour_button.cpp      | 11 ++++++++---
 aegisub/src/dialog_colorpicker.cpp |  2 ++
 aegisub/src/dialog_colorpicker.h   |  4 ++--
 aegisub/src/subs_edit_box.cpp      | 11 +++++------
 4 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/aegisub/src/colour_button.cpp b/aegisub/src/colour_button.cpp
index a71c9235b..f2c0e18dc 100644
--- a/aegisub/src/colour_button.cpp
+++ b/aegisub/src/colour_button.cpp
@@ -65,6 +65,8 @@ ColourButton::~ColourButton() {
 /// @brief Callback for the color picker dialog
 /// @param col New color
 void ColourButton::SetColour(wxColour col) {
+	if (!col.IsOk()) return;
+
 	colour = col;
 
 	// Draw colour
@@ -91,8 +93,11 @@ wxColour ColourButton::GetColour() {
 /// @brief Click handler
 /// @param event
 void ColourButton::OnClick(wxCommandEvent &event) {
-	if (event.GetClientData() != this)
-		GetColorFromUser<ColourButton, &ColourButton::SetColour>(GetParent(), colour, this);
-	else
+	if (event.GetClientData() == this)
 		event.Skip();
+	else {
+		wxColour initial = colour;
+		if (!GetColorFromUser<ColourButton, &ColourButton::SetColour>(GetParent(), colour, this).IsOk())
+			SetColour(initial);
+	}
 }
diff --git a/aegisub/src/dialog_colorpicker.cpp b/aegisub/src/dialog_colorpicker.cpp
index 3f3398da4..da9d702df 100644
--- a/aegisub/src/dialog_colorpicker.cpp
+++ b/aegisub/src/dialog_colorpicker.cpp
@@ -633,6 +633,8 @@ wxColour GetColorFromUser(wxWindow *parent, wxColour original, ColorCallback cal
 	DialogColorPicker dialog(parent, original, callback, userdata);
 	if (dialog.ShowModal() == wxID_OK)
 		original = dialog.GetColor();
+	else
+		original = wxNullColour;
 	if (callback)
 		callback(userdata, original);
 	return original;
diff --git a/aegisub/src/dialog_colorpicker.h b/aegisub/src/dialog_colorpicker.h
index a29865d4d..77a3b91ea 100644
--- a/aegisub/src/dialog_colorpicker.h
+++ b/aegisub/src/dialog_colorpicker.h
@@ -52,7 +52,7 @@ void ColorCallbackWrapper(void* obj, wxColor color) {
 /// @param original Initial color to select
 /// @param callback Function called whenever the selected color changes if not NULL
 /// @param userdata Passed to callback function
-/// @return Last selected color when dialog is closed, or original if the dialog was cancelled
+/// @return Last selected color when dialog is closed, or wxNullColour if the dialog was cancelled
 wxColor GetColorFromUser(wxWindow* parent, wxColor original, ColorCallback callback = NULL, void* userdata = NULL);
 
 /// @brief Get a color from the user via a color picker dialog
@@ -61,7 +61,7 @@ wxColor GetColorFromUser(wxWindow* parent, wxColor original, ColorCallback callb
 /// @param parent Parent window
 /// @param original Initial color to select
 /// @param callbackObj Object to call callback method on. Must be of type T.
-/// @return Last selected color when dialog is closed, or original if the dialog was cancelled
+/// @return Last selected color when dialog is closed, or wxNullColour if the dialog was cancelled
 template<class T, void (T::*method)(wxColor)>
 wxColor GetColorFromUser(wxWindow* parent, wxColor original, T* callbackObj) {
 	return GetColorFromUser(parent, original, &ColorCallbackWrapper<T, method>, callbackObj);
diff --git a/aegisub/src/subs_edit_box.cpp b/aegisub/src/subs_edit_box.cpp
index cb65b05b7..8b5b67d6e 100644
--- a/aegisub/src/subs_edit_box.cpp
+++ b/aegisub/src/subs_edit_box.cpp
@@ -722,15 +722,14 @@ void SubsEditBox::OnColorButton(AssColor (AssStyle::*field), const char *tag, co
 	int blockn = block_at_pos(line->Text, sel.first);
 
 	color = get_value(*line, blockn, color, colorTag, alt);
-	wxString initialText = line->Text;
 	wxColor newColor = GetColorFromUser<SubsEditBox, &SubsEditBox::SetColorCallback>(c->parent, color, this);
-	if (newColor == color) {
-		TextEdit->SetTextTo(initialText);
-		TextEdit->SetSelectionU(sel.first, sel.second);
-	}
-
 	line->ClearBlocks();
 	CommitText(_("set color"));
+
+	if (!newColor.IsOk()) {
+		c->ass->Undo();
+		TextEdit->SetSelectionU(sel.first, sel.second);
+	}
 }
 
 void SubsEditBox::SetColorCallback(wxColor newColor) {