[geary/bug/728002-webkit2: 75/140] Fix infinite WebView key event chain when it doesn't handle a key press.

[Michael Gratton <mjog src gnome org>]

commit a8d1da7ee78ba5b7fd048b9c7e95d5dffeab7fc5
Author: Michael James Gratton <mike vee net>
Date:   Fri Jan 6 00:18:08 2017 +1100

    Fix infinite WebView key event chain when it doesn't handle a key press.
    
    * src/client/components/main-window.vala (MainWindow): Convert
      on_key_press_event handler to override Gtk.Window's key_press_event
      virtual method. Reimplement that so that single keystroke shortcuts
      work, but also so GtkWindow.propagate_key_event only gets called once,
      and the WebKit event loop is avoided. See source comments for
      details. Also clean up shift up/down handling to not update when any
      GtkEntry or ComposerWebView is focused.

 src/client/components/main-window.vala |   79 ++++++++++++++++++++++++-------
 ui/main-window.ui                      |    1 -
 2 files changed, 61 insertions(+), 19 deletions(-)
---
diff --git a/src/client/components/main-window.vala b/src/client/components/main-window.vala
index 8e22550..165e6c4 100644
--- a/src/client/components/main-window.vala
+++ b/src/client/components/main-window.vala
@@ -215,6 +215,54 @@ public class MainWindow : Gtk.ApplicationWindow {
         return scrollbar != null && scrollbar.get_visible();
     }
 
+    public override bool key_press_event(Gdk.EventKey event) {
+        check_shift_event(event);
+
+        /* Ensure that single-key command (SKC) shortcuts don't
+         * interfere with text input.
+         *
+         * The default GtkWindow::key_press_event implementation calls
+         * gtk_window_activate_key -- which would activate the SKC,
+         * before calling gtk_window_propagate_key_event -- which
+         * would send the event to any focused text entry control, so
+         * we need to override that. A quick hack is to just call
+         * gtk_window_propagate_key_event here, then chain up. But
+         * that means two calls to that method for every key press,
+         * which in the worst case means all widgets in the focus
+         * chain would be consulted to handle the press twice, which
+         * sucks.
+         *
+         * Worse however, is that due to WK2 Bug 136430[0], WebView
+         * instances duplicate any key events they don't handle. For
+         * the editor, that means simple key presses like 'a' will
+         * only result in a single event, since the web view adds the
+         * letter to the document. But if not handled, e.g. when the
+         * user presses Shift, Ctrl, or similar, then it also produces
+         * a second event. Combined with the
+         * gtk_window_propagate_key_event above, this leads to a
+         * cambrian explosion of key events - an exponential number
+         * are generated, which is bad. This problem also applies to
+         * ConversationWebView instances, since none of them handle
+         * events.
+         *
+         * The work around here is completely override the default
+         * implementation to reverse it. So if something related to
+         * key handling breaks in the future, this might be a good
+         * place to start looking. Better alternatives welcome.
+         *
+         * [0] - <https://bugs.webkit.org/show_bug.cgi?id=136430>
+         */
+
+        bool handled = propagate_key_event(event);
+        if (!handled) {
+            handled = activate_key(event);
+        }
+        if (!handled) {
+            handled = Gtk.bindings_activate_event(this, event);
+        }
+        return handled;
+    }
+
     private void on_conversation_monitor_changed() {
         ConversationListStore? old_model = this.conversation_list_view.get_model();
         if (old_model != null) {
@@ -343,28 +391,23 @@ public class MainWindow : Gtk.ApplicationWindow {
             this.main_toolbar.folder = this.current_folder.get_display_name();
     }
 
-    [GtkCallback]
-    private bool on_key_press_event(Gdk.EventKey event) {
-        if ((event.keyval == Gdk.Key.Shift_L || event.keyval == Gdk.Key.Shift_R)
-            && (event.state & Gdk.ModifierType.SHIFT_MASK) == 0
-            && !this.search_bar.search_entry_has_focus)
-            on_shift_key(true);
-
-        // Check whether the focused widget wants to handle it, if not let the accelerators kick in
-        // via the default handling
-        return propagate_key_event(event);
-    }
-
-    [GtkCallback]
-    private bool on_key_release_event(Gdk.EventKey event) {
+    private inline void check_shift_event(Gdk.EventKey event) {
         // FIXME: it's possible the user will press two shift keys.  We want
         // the shift key to report as released when they release ALL of them.
         // There doesn't seem to be an easy way to do this in Gdk.
-        if ((event.keyval == Gdk.Key.Shift_L || event.keyval == Gdk.Key.Shift_R)
-            && !this.search_bar.search_entry_has_focus)
-            on_shift_key(false);
+        if (event.keyval == Gdk.Key.Shift_L || event.keyval == Gdk.Key.Shift_R) {
+            Gtk.Widget? focus = get_focus();
+            if (focus == null ||
+                (!(focus is Gtk.Entry) && !(focus is ComposerWebView))) {
+                on_shift_key(event.type == Gdk.EventType.KEY_PRESS);
+            }
+        }
+    }
 
-        return propagate_key_event(event);
+    [GtkCallback]
+    private bool on_key_release_event(Gdk.EventKey event) {
+        check_shift_event(event);
+        return Gdk.EVENT_PROPAGATE;
     }
 
     [GtkCallback]
diff --git a/ui/main-window.ui b/ui/main-window.ui
index a24070c..2bc3833 100644
--- a/ui/main-window.ui
+++ b/ui/main-window.ui
@@ -6,7 +6,6 @@
     <property name="show_menubar">False</property>
     <property name="events">GDK_KEY_PRESS_MASK | GDK_KEY_RELEASE_MASK | GDK_FOCUS_CHANGE_MASK | 
GDK_STRUCTURE_MASK</property>
     <signal name="delete_event" handler="on_delete_event"/>
-    <signal name="key_press_event" handler="on_key_press_event"/>
     <signal name="key_release_event" handler="on_key_release_event"/>
     <signal name="focus_in_event" handler="on_focus_event"/>
     <child>