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
--- 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();