Bug 1278581 - Part 2 - Ensure a stray key-up event doesn't end up in Gecko after committing the URL bar input. r?jchen draft
authorJan Henning <jh+bugzilla@buttercookie.de>
Wed, 21 Mar 2018 20:44:31 +0100
changeset 775264 99d86f385510fd7a50cba892ab2601d53c25fd9f
parent 775255 edc27e880dc2f7cde68dccf6081127cacf10c5e7
child 775265 c1bb1f245f7578f260eeab230a479539c01c5ecc
push id104675
push usermozilla@buttercookie.de
push dateFri, 30 Mar 2018 19:09:15 +0000
reviewersjchen
bugs1278581
milestone61.0a1
Bug 1278581 - Part 2 - Ensure a stray key-up event doesn't end up in Gecko after committing the URL bar input. r?jchen When editing the URL bar, we're committing the input and starting a page load in response to a key-down event for the Enter key. We end up hiding the ToolbarEditLayout from within the key event handler, which means that by the time we get the corresponding key-up event, input focus has moved on and the key event ends up being dispatched to the GeckoView and consequently to Gecko. This means that loading an URL from the URL bar by pressing Enter will reset millisSinceLastUserInput, which defeats our logic to distinguish URLs loaded via the Android UI from those loaded by clicking on links within the content. MozReview-Commit-ID: 3Wbnk3bnqVS
mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
mobile/android/base/java/org/mozilla/gecko/toolbar/BrowserToolbar.java
mobile/android/base/java/org/mozilla/gecko/toolbar/ToolbarEditText.java
--- a/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
@@ -305,16 +305,18 @@ public class BrowserApp extends GeckoApp
 
     private ExtensionPermissionsHelper mExtensionPermissionsHelper;
 
     // The tab to be selected on editing mode exit.
     private Integer mTargetTabForEditingMode;
 
     private final TabEditingState mLastTabEditingState = new TabEditingState();
 
+    private boolean mSuppressNextKeyUp;
+
     // The animator used to toggle HomePager visibility has a race where if the HomePager is shown
     // (starting the animation), the HomePager is hidden, and the HomePager animation completes,
     // both the web content and the HomePager will be hidden. This flag is used to prevent the
     // race by determining if the web content should be hidden at the animation's end.
     private boolean mHideWebContentOnAnimationEnd;
 
     private final DynamicToolbar mDynamicToolbar = new DynamicToolbar();
 
@@ -479,16 +481,20 @@ public class BrowserApp extends GeckoApp
     public boolean onKey(View v, int keyCode, KeyEvent event) {
         if (AndroidGamepadManager.handleKeyEvent(event)) {
             return true;
         }
 
         // Global onKey handler. This is called if the focused UI doesn't
         // handle the key event, and before Gecko swallows the events.
         if (event.getAction() != KeyEvent.ACTION_DOWN) {
+            if (mSuppressNextKeyUp && event.getAction() == KeyEvent.ACTION_UP) {
+                mSuppressNextKeyUp = false;
+                return true;
+            }
             return false;
         }
 
         if ((event.getSource() & InputDevice.SOURCE_GAMEPAD) == InputDevice.SOURCE_GAMEPAD) {
             switch (keyCode) {
                 case KeyEvent.KEYCODE_BUTTON_Y:
                     // Toggle/focus the address bar on gamepad-y button.
                     if (mBrowserChrome.getVisibility() == View.VISIBLE) {
@@ -1276,18 +1282,23 @@ public class BrowserApp extends GeckoApp
             @Override
             public void onActivate() {
                 enterEditingMode();
             }
         });
 
         mBrowserToolbar.setOnCommitListener(new BrowserToolbar.OnCommitListener() {
             @Override
-            public void onCommit() {
-                commitEditingMode();
+            public void onCommitByKey() {
+                if (commitEditingMode()) {
+                    // We're committing in response to a key-down event. Since we'll be hiding the
+                    // ToolbarEditLayout, the corresponding key-up event will end up being sent to
+                    // Gecko which we don't want, as this messes up tracking of the last user input.
+                    mSuppressNextKeyUp = true;
+                }
             }
         });
 
         mBrowserToolbar.setOnDismissListener(new BrowserToolbar.OnDismissListener() {
             @Override
             public void onDismiss() {
                 mBrowserToolbar.cancelEdit();
             }
@@ -2506,19 +2517,22 @@ public class BrowserApp extends GeckoApp
         mBrowserToolbar.startEditing(url, animator);
 
         showHomePagerWithAnimator(panelId, null, animator);
 
         animator.start();
         Telemetry.startUISession(TelemetryContract.Session.AWESOMESCREEN);
     }
 
-    private void commitEditingMode() {
+    /**
+     * @return True if editing mode was successfully committed.
+     */
+    private boolean commitEditingMode() {
         if (!mBrowserToolbar.isEditing()) {
-            return;
+            return false;
         }
 
         Telemetry.stopUISession(TelemetryContract.Session.AWESOMESCREEN,
                                 TelemetryContract.Reason.COMMIT);
 
         final String url = mBrowserToolbar.commitEdit();
 
         // HACK: We don't know the url that will be loaded when hideHomePager is initially called
@@ -2530,16 +2544,18 @@ public class BrowserApp extends GeckoApp
         //
         // Here we call hideHomePager for the second time with the URL to be loaded so that
         // hideHomePager is called with the correct state for the upcoming page load.
         //
         // Expected to be fixed by bug 915825.
         hideHomePager(url);
         loadUrlOrKeywordSearch(url);
         clearSelectedTabApplicationId();
+
+        return true;
     }
 
     private void clearSelectedTabApplicationId() {
         final Tab selected = Tabs.getInstance().getSelectedTab();
         if (selected != null) {
             selected.setApplicationId(null);
         }
     }
--- a/mobile/android/base/java/org/mozilla/gecko/toolbar/BrowserToolbar.java
+++ b/mobile/android/base/java/org/mozilla/gecko/toolbar/BrowserToolbar.java
@@ -89,17 +89,17 @@ public abstract class BrowserToolbar ext
     private static final int LIGHTWEIGHT_THEME_INVERT_ALPHA_END = 179;
     public static final int LIGHTWEIGHT_THEME_INVERT_ALPHA_TABLET = 51;
 
     public interface OnActivateListener {
         public void onActivate();
     }
 
     public interface OnCommitListener {
-        public void onCommit();
+        public void onCommitByKey();
     }
 
     public interface OnDismissListener {
         public void onDismiss();
     }
 
     public interface OnFilterListener {
         public void onFilter(String searchText, AutocompleteHandler handler);
--- a/mobile/android/base/java/org/mozilla/gecko/toolbar/ToolbarEditText.java
+++ b/mobile/android/base/java/org/mozilla/gecko/toolbar/ToolbarEditText.java
@@ -592,17 +592,17 @@ public class ToolbarEditText extends Cus
             }
 
             if (keyCode == KeyEvent.KEYCODE_ENTER) {
                 // If the edit text has a composition string, don't submit the text yet.
                 // ENTER is needed to commit the composition string.
                 final Editable content = getText();
                 if (!hasCompositionString(content)) {
                     if (mCommitListener != null) {
-                        mCommitListener.onCommit();
+                        mCommitListener.onCommitByKey();
                     }
 
                     return true;
                 }
             }
 
             if (keyCode == KeyEvent.KEYCODE_BACK) {
                 // Drop the virtual keyboard.
@@ -618,17 +618,17 @@ public class ToolbarEditText extends Cus
         @Override
         public boolean onKey(View v, int keyCode, KeyEvent event) {
             if (keyCode == KeyEvent.KEYCODE_ENTER || GamepadUtils.isActionKey(event)) {
                 if (event.getAction() != KeyEvent.ACTION_DOWN) {
                     return true;
                 }
 
                 if (mCommitListener != null) {
-                    mCommitListener.onCommit();
+                    mCommitListener.onCommitByKey();
                 }
 
                 return true;
             }
 
             if (GamepadUtils.isBackKey(event)) {
                 if (mDismissListener != null) {
                     mDismissListener.onDismiss();