From 9eead5d14507b6543d957a59ff16b68bab6d61cd Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Wed, 13 Feb 2019 22:19:44 +1100 Subject: [PATCH] Replace composer keyboard shortcut with standard widget action scoping Rather than adding and removing shortcuts when the composer's web view gains and loses focus to avoid invoking main window shortcuts, just rely on standard GTK action scoping rules. This fixes editing shortcuts like Ctrl+I breaking when clicking on toolbar buttons, a crasher because the composer left the undo button enabled after it was closed, and allows Escape to close the composer from anywherre. Fixes https://bugzilla.gnome.org/show_bug.cgi?id=774651, https://bugzilla.gnome.org/show_bug.cgi?id=785187, and https://bugzilla.gnome.org/show_bug.cgi?id=741741 --- src/client/application/geary-application.vala | 11 ++- src/client/composer/composer-box.vala | 11 +-- src/client/composer/composer-container.vala | 77 ------------------- src/client/composer/composer-embed.vala | 11 --- src/client/composer/composer-widget.vala | 52 +++++++------ src/client/composer/composer-window.vala | 6 -- ui/composer-menus.ui | 44 +++++------ ui/composer-widget.ui | 56 ++++++++++---- 8 files changed, 99 insertions(+), 169 deletions(-) diff --git a/src/client/application/geary-application.vala b/src/client/application/geary-application.vala index a589ad17..79740282 100644 --- a/src/client/application/geary-application.vala +++ b/src/client/application/geary-application.vala @@ -235,10 +235,11 @@ public class GearyApplication : Gtk.Application { // the other instances called when sending commands to the app via the command-line) message("%s %s prefix=%s exec_dir=%s is_installed=%s", NAME, VERSION, INSTALL_PREFIX, exec_dir.get_path(), is_installed().to_string()); - + config = new Configuration(APP_ID); + ComposerWidget.add_window_accelerators(this); yield controller.open_async(null); - + release(); } @@ -253,6 +254,12 @@ public class GearyApplication : Gtk.Application { is_destroyed = true; } + public void add_window_accelerators(string action, + string[] accelerators, + Variant? param = null) { + set_accels_for_action("win." + action, accelerators); + } + public void show_accounts() { activate(); diff --git a/src/client/composer/composer-box.vala b/src/client/composer/composer-box.vala index e3116d88..9bf9f3e9 100644 --- a/src/client/composer/composer-box.vala +++ b/src/client/composer/composer-box.vala @@ -37,17 +37,10 @@ public class ComposerBox : Gtk.Frame, ComposerContainer { add(this.composer); this.main_toolbar.set_conversation_header(composer.header); - this.composer.editor.focus_in_event.connect(on_focus_in); - this.composer.editor.focus_out_event.connect(on_focus_out); show(); } public void remove_composer() { - if (this.composer.editor.has_focus) - on_focus_out(); - this.composer.editor.focus_in_event.disconnect(on_focus_in); - this.composer.editor.focus_out_event.disconnect(on_focus_out); - remove(this.composer); close_container(); } @@ -56,8 +49,6 @@ public class ComposerBox : Gtk.Frame, ComposerContainer { hide(); this.main_toolbar.remove_conversation_header(composer.header); this.composer.state = ComposerWidget.ComposerState.DETACHED; - this.composer.editor.focus_in_event.disconnect(on_focus_in); - this.composer.editor.focus_out_event.disconnect(on_focus_out); vanished(); } @@ -66,5 +57,5 @@ public class ComposerBox : Gtk.Frame, ComposerContainer { vanish(); destroy(); } -} +} diff --git a/src/client/composer/composer-container.vala b/src/client/composer/composer-container.vala index c96a9e51..466e8166 100644 --- a/src/client/composer/composer-container.vala +++ b/src/client/composer/composer-container.vala @@ -44,81 +44,4 @@ public interface ComposerContainer { */ public abstract void remove_composer(); - protected virtual bool on_focus_in() { - if (this.old_accelerators == null) { - this.old_accelerators = new Gee.HashMultiMap(); - add_accelerators(); - } - return false; - } - - protected virtual bool on_focus_out() { - if (this.old_accelerators != null) { - remove_accelerators(); - this.old_accelerators = null; - } - return false; - } - - /** - * Adds the accelerators for the child composer, and temporarily removes conflicting - * accelerators from existing actions. - */ - protected virtual void add_accelerators() { - GearyApplication app = GearyApplication.instance; - - // Check for actions with conflicting accelerators - foreach (string action in ComposerWidget.action_accelerators.get_keys()) { - foreach (string accelerator in ComposerWidget.action_accelerators[action]) { - string[] actions = app.get_actions_for_accel(accelerator); - - foreach (string conflicting_action in actions) { - remove_conflicting_accelerator(conflicting_action, accelerator); - this.old_accelerators[conflicting_action] = accelerator; - } - } - } - - // Now add our actions to the window and their accelerators - foreach (string action in ComposerWidget.action_accelerators.get_keys()) { - this.top_window.add_action(composer.get_action(action)); - app.set_accels_for_action("win." + action, - ComposerWidget.action_accelerators[action].to_array()); - } - } - - /** - * Removes the accelerators for the child composer, and restores previously removed accelerators. - */ - protected virtual void remove_accelerators() { - foreach (string action in ComposerWidget.action_accelerators.get_keys()) - GearyApplication.instance.set_accels_for_action("win." + action, {}); - - foreach (string action in old_accelerators.get_keys()) - foreach (string accelerator in this.old_accelerators[action]) - restore_conflicting_accelerator(action, accelerator); - } - - // Helper method. Removes the given conflicting accelerator from the action's accelerators. - private void remove_conflicting_accelerator(string action, string accelerator) { - GearyApplication app = GearyApplication.instance; - string[] accelerators = app.get_accels_for_action(action); - if (accelerators.length == 0) - return; - - string[] without_accel = new string[accelerators.length - 1]; - foreach (string a in accelerators) - if (a != accelerator) - without_accel += a; - - app.set_accels_for_action(action, without_accel); - } - - // Helper method. Adds the given accelerator back to the action's accelerators. - private void restore_conflicting_accelerator(string action, string accelerator) { - GearyApplication app = GearyApplication.instance; - string[] accelerators = app.get_accels_for_action(action); - accelerators += accelerator; - app.set_accels_for_action(action, accelerators); - } } diff --git a/src/client/composer/composer-embed.vala b/src/client/composer/composer-embed.vala index 460e164e..ce2390e4 100644 --- a/src/client/composer/composer-embed.vala +++ b/src/client/composer/composer-embed.vala @@ -42,8 +42,6 @@ public class ComposerEmbed : Gtk.EventBox, ComposerContainer { add(composer); realize.connect(on_realize); - this.composer.editor.focus_in_event.connect(on_focus_in); - this.composer.editor.focus_out_event.connect(on_focus_out); show(); } @@ -71,14 +69,7 @@ public class ComposerEmbed : Gtk.EventBox, ComposerContainer { } public void remove_composer() { - if (this.composer.editor.has_focus) - on_focus_out(); - - this.composer.editor.focus_in_event.disconnect(on_focus_in); - this.composer.editor.focus_out_event.disconnect(on_focus_out); - disable_scroll_reroute(this); - remove(this.composer); close_container(); } @@ -189,8 +180,6 @@ public class ComposerEmbed : Gtk.EventBox, ComposerContainer { public void vanish() { hide(); this.composer.state = ComposerWidget.ComposerState.DETACHED; - this.composer.editor.focus_in_event.disconnect(on_focus_in); - this.composer.editor.focus_out_event.disconnect(on_focus_out); vanished(); } diff --git a/src/client/composer/composer-widget.vala b/src/client/composer/composer-widget.vala index 0e3e16c0..754ee573 100644 --- a/src/client/composer/composer-widget.vala +++ b/src/client/composer/composer-widget.vala @@ -47,8 +47,6 @@ public class ComposerWidget : Gtk.EventBox, Geary.BaseInterface { } } - private SimpleActionGroup actions = new SimpleActionGroup(); - private const string ACTION_UNDO = "undo"; private const string ACTION_REDO = "redo"; private const string ACTION_CUT = "cut"; @@ -90,7 +88,7 @@ public class ComposerWidget : Gtk.EventBox, Geary.BaseInterface { ACTION_BOLD, ACTION_ITALIC, ACTION_UNDERLINE, ACTION_STRIKETHROUGH, ACTION_FONT_SIZE, ACTION_FONT_FAMILY, ACTION_COLOR, ACTION_JUSTIFY, ACTION_INSERT_IMAGE, ACTION_COPY_LINK, - ACTION_OLIST, ACTION_ULIST + ACTION_OLIST, ACTION_ULIST }; private const ActionEntry[] action_entries = { @@ -133,26 +131,26 @@ public class ComposerWidget : Gtk.EventBox, Geary.BaseInterface { }; public static Gee.MultiMap action_accelerators = new Gee.HashMultiMap(); - static construct { - action_accelerators.set(ACTION_UNDO, "z"); - action_accelerators.set(ACTION_REDO, "z"); - action_accelerators.set(ACTION_CUT, "x"); - action_accelerators.set(ACTION_COPY, "c"); - action_accelerators.set(ACTION_PASTE, "v"); - action_accelerators.set(ACTION_PASTE_WITHOUT_FORMATTING, "v"); - action_accelerators.set(ACTION_INSERT_IMAGE, "g"); - action_accelerators.set(ACTION_INSERT_LINK, "l"); - action_accelerators.set(ACTION_INDENT, "bracketright"); - action_accelerators.set(ACTION_OUTDENT, "bracketleft"); - action_accelerators.set(ACTION_REMOVE_FORMAT, "space"); - action_accelerators.set(ACTION_BOLD, "b"); - action_accelerators.set(ACTION_ITALIC, "i"); - action_accelerators.set(ACTION_UNDERLINE, "u"); - action_accelerators.set(ACTION_STRIKETHROUGH, "k"); - action_accelerators.set(ACTION_CLOSE, "w"); - action_accelerators.set(ACTION_CLOSE, "Escape"); - action_accelerators.set(ACTION_ADD_ATTACHMENT, "t"); - action_accelerators.set(ACTION_DETACH, "d"); + + public static void add_window_accelerators(GearyApplication application) { + application.add_window_accelerators(ACTION_UNDO, { "z" } ); + application.add_window_accelerators(ACTION_REDO, { "z" } ); + application.add_window_accelerators(ACTION_CUT, { "x" } ); + application.add_window_accelerators(ACTION_COPY, { "c" } ); + application.add_window_accelerators(ACTION_PASTE, { "v" } ); + application.add_window_accelerators(ACTION_PASTE_WITHOUT_FORMATTING, { "v" } ); + application.add_window_accelerators(ACTION_INSERT_IMAGE, { "g" } ); + application.add_window_accelerators(ACTION_INSERT_LINK, { "l" } ); + application.add_window_accelerators(ACTION_INDENT, { "bracketright" } ); + application.add_window_accelerators(ACTION_OUTDENT, { "bracketleft" } ); + application.add_window_accelerators(ACTION_REMOVE_FORMAT, { "space" } ); + application.add_window_accelerators(ACTION_BOLD, { "b" } ); + application.add_window_accelerators(ACTION_ITALIC, { "i" } ); + application.add_window_accelerators(ACTION_UNDERLINE, { "u" } ); + application.add_window_accelerators(ACTION_STRIKETHROUGH, { "k" } ); + application.add_window_accelerators(ACTION_CLOSE, { "w", "Escape" } ); + application.add_window_accelerators(ACTION_ADD_ATTACHMENT, { "t" } ); + application.add_window_accelerators(ACTION_DETACH, { "d" } ); } private const string DRAFT_SAVED_TEXT = _("Saved"); @@ -322,6 +320,8 @@ public class ComposerWidget : Gtk.EventBox, Geary.BaseInterface { [GtkChild] private Gtk.Box message_area; + private SimpleActionGroup actions = new SimpleActionGroup(); + private Menu html_menu; private Menu plain_menu; @@ -809,8 +809,10 @@ public class ComposerWidget : Gtk.EventBox, Geary.BaseInterface { private void initialize_actions() { this.actions.add_action_entries(action_entries, this); - // for some reason, we can't use the same prefix. - insert_action_group("cmp", this.actions); + // Main actions should use 'win' prefix so they override main + // window action. But for some reason, we can't use the same + // prefix for the headerbar. + insert_action_group("win", this.actions); this.header.insert_action_group("cmh", this.actions); this.actions.change_action_state( diff --git a/src/client/composer/composer-window.vala b/src/client/composer/composer-window.vala index 634ba095..d11fbcb1 100644 --- a/src/client/composer/composer-window.vala +++ b/src/client/composer/composer-window.vala @@ -36,8 +36,6 @@ public class ComposerWindow : Gtk.ApplicationWindow, ComposerContainer { set_property("name", "GearyComposerWindow"); add(this.composer); - focus_in_event.connect(on_focus_in); - focus_out_event.connect(on_focus_out); if (composer.config.desktop_environment == Configuration.DesktopEnvironment.UNITY) { composer.embed_header(); @@ -105,10 +103,6 @@ public class ComposerWindow : Gtk.ApplicationWindow, ComposerContainer { } public void close_container() { - on_focus_out(); - this.composer.editor.focus_in_event.disconnect(on_focus_in); - this.composer.editor.focus_out_event.disconnect(on_focus_out); - this.closing = true; destroy(); } diff --git a/ui/composer-menus.ui b/ui/composer-menus.ui index 72f5b37c..e6854a4e 100644 --- a/ui/composer-menus.ui +++ b/ui/composer-menus.ui @@ -5,53 +5,53 @@
S_ans Serif - cmp.font-family + win.font-family sans S_erif - cmp.font-family + win.font-family serif _Fixed Width - cmp.font-family + win.font-family monospace
_Small - cmp.font-size + win.font-size small _Medium - cmp.font-size + win.font-size medium Lar_ge - cmp.font-size + win.font-size large
C_olor - cmp.color + win.color
_Rich Text - cmp.compose-as-html + win.compose-as-html
Show Extended Fields - cmp.show-extended + win.show-extended
@@ -60,13 +60,13 @@
_Rich Text - cmp.compose-as-html + win.compose-as-html
Show Extended Fields - cmp.show-extended + win.show-extended
@@ -76,56 +76,56 @@
_Undo - cmp.undo + win.undo _Redo - cmp.redo + win.redo
Cu_t - cmp.cut + win.cut _Copy - cmp.copy + win.copy _Paste - cmp.paste + win.paste Paste _Without Formatting - cmp.paste-without-formatting + win.paste-without-formatting
Cu_t - cmp.cut + win.cut _Copy - cmp.copy + win.copy _Paste - cmp.paste + win.paste
Select _All - cmp.select-all + win.select-all
_Inspect… - cmp.open_inspector + win.open_inspector
diff --git a/ui/composer-widget.ui b/ui/composer-widget.ui index 4414f4fb..d22e881c 100644 --- a/ui/composer-widget.ui +++ b/ui/composer-widget.ui @@ -1,5 +1,5 @@ - +