Bug 1252310 - Don't mistakenly suppress key-up event when not required. r=geckoview-reviewers,m_kato
authorJan Henning <jh+bitbucket@buttercookie.de>
Mon, 13 May 2019 06:48:30 +0000
changeset 532399 c7feffe40029e833241baa952cec89346d649a06
parent 532398 5c99c6daa9db60f35fb85956271fd855e7c0cd7b
child 532400 fbc8b6af3e5499c317e3d481f44ca84f77c459a9
push id11268
push usercsabou@mozilla.com
push dateTue, 14 May 2019 15:24:22 +0000
treeherdermozilla-beta@5fb7fcd568d6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgeckoview-reviewers, m_kato
bugs1252310, 1278581
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1252310 - Don't mistakenly suppress key-up event when not required. r=geckoview-reviewers,m_kato The problem from bug 1278581 was that hiding the URL bar in response to a key-down event (for the Enter key) would then lead to the corresponding key-up event then ending up in GeckoView, thereby confusing the "last user activity" tracking. It appears that this only happens with key events received through the regular OnKeyListener, but not with events coming from the OnKeyPreImeListener. On devices where pressing Enter in the URL bar would transmit the key event through the latter mechanism, this means that because the key-up event we wanted to suppress in BrowserApp never arrived, we would instead suppress whatever other key event would arrive next, e.g. possibly a press of the back button. This would lead to the observed behaviour where after entering an URL, the first subsequent press of the back button might then be ignored. Differential Revision: https://phabricator.services.mozilla.com/D30573
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
@@ -135,16 +135,17 @@ import org.mozilla.gecko.tabs.TabHistory
 import org.mozilla.gecko.tabs.TabHistoryFragment;
 import org.mozilla.gecko.tabs.TabHistoryPage;
 import org.mozilla.gecko.tabs.TabsPanel;
 import org.mozilla.gecko.telemetry.TelemetryCorePingDelegate;
 import org.mozilla.gecko.telemetry.TelemetryUploadService;
 import org.mozilla.gecko.telemetry.measurements.SearchCountMeasurements;
 import org.mozilla.gecko.toolbar.AutocompleteHandler;
 import org.mozilla.gecko.toolbar.BrowserToolbar;
+import org.mozilla.gecko.toolbar.BrowserToolbar.CommitEventSource;
 import org.mozilla.gecko.toolbar.BrowserToolbar.TabEditingState;
 import org.mozilla.gecko.toolbar.PwaConfirm;
 import org.mozilla.gecko.trackingprotection.TrackingProtectionPrompt;
 import org.mozilla.gecko.updater.PostUpdateHandler;
 import org.mozilla.gecko.updater.UpdateServiceHelper;
 import org.mozilla.gecko.util.ActivityUtils;
 import org.mozilla.gecko.util.ContextUtils;
 import org.mozilla.gecko.util.DrawableUtil;
@@ -1253,18 +1254,19 @@ public class BrowserApp extends GeckoApp
             @Override
             public void onActivate() {
                 enterEditingMode();
             }
         });
 
         mBrowserToolbar.setOnCommitListener(new BrowserToolbar.OnCommitListener() {
             @Override
-            public void onCommitByKey() {
-                if (commitEditingMode()) {
+            public void onCommit(CommitEventSource eventSource) {
+                final boolean didCommit = commitEditingMode();
+                if (didCommit && eventSource == CommitEventSource.KEY_EVENT) {
                     // 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;
                 }
             }
         });
 
--- 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,22 @@ 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 onCommitByKey();
+        public void onCommit(CommitEventSource eventSource);
+    }
+
+    public enum CommitEventSource {
+        KEY_EVENT,
+        PRE_IME_KEY_EVENT
     }
 
     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
@@ -3,16 +3,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.toolbar;
 
 import org.mozilla.gecko.AboutPages;
 import org.mozilla.gecko.CustomEditText;
 import org.mozilla.gecko.InputMethods;
+import org.mozilla.gecko.toolbar.BrowserToolbar.CommitEventSource;
 import org.mozilla.gecko.toolbar.BrowserToolbar.OnCommitListener;
 import org.mozilla.gecko.toolbar.BrowserToolbar.OnDismissListener;
 import org.mozilla.gecko.toolbar.BrowserToolbar.OnFilterListener;
 import org.mozilla.gecko.toolbar.ToolbarEditLayout.OnSearchStateChangeListener;
 import org.mozilla.gecko.util.GamepadUtils;
 import org.mozilla.gecko.util.StringUtils;
 
 import android.content.Context;
@@ -600,17 +601,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.onCommitByKey();
+                        mCommitListener.onCommit(CommitEventSource.PRE_IME_KEY_EVENT);
                     }
 
                     return true;
                 }
             }
 
             if (keyCode == KeyEvent.KEYCODE_BACK) {
                 // Drop the virtual keyboard.
@@ -626,17 +627,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.onCommitByKey();
+                    mCommitListener.onCommit(CommitEventSource.KEY_EVENT);
                 }
 
                 return true;
             }
 
             if (GamepadUtils.isBackKey(event)) {
                 if (mDismissListener != null) {
                     mDismissListener.onDismiss();