Commit 76dfa923 authored by Tomasz Wiszkowski's avatar Tomasz Wiszkowski Committed by Chromium LUCI CQ

Fix keyboard not hiding when overflow menu is opened.

This change addresses the scenario where:
- the user is on a website with an input field
- the input field (not the Omnibox) is focused, raising keyboard,
- the user next presses the overflow menu.

In this situation, the keyboard should be dismissed.

Bug: 1160617
Change-Id: I84716a264b20c2a57dc6c836325a6d94e14163dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2613516Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Reviewed-by: default avatarPatrick Noland <pnoland@chromium.org>
Commit-Queue: Tomasz Wiszkowski <ender@google.com>
Cr-Commit-Position: refs/heads/master@{#842101}
parent a842f355
......@@ -5,6 +5,7 @@ include_rules = [
"+base/android/java/src/org/chromium/base/supplier",
"+chrome/android/java/src/org/chromium/chrome/browser/toolbar",
"+chrome/android/java/src/org/chromium/chrome/browser/omnibox",
"+ui/android/java/src/org/chromium/ui",
"+ui/android/java/src/org/chromium/ui/base",
]
......
......@@ -438,14 +438,14 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve
Runnable requestFocusRunnable = compositorViewHolder::requestFocus;
mMenuButtonCoordinator = new MenuButtonCoordinator(appMenuCoordinatorSupplier,
mControlsVisibilityDelegate, mActivity,
mControlsVisibilityDelegate, mWindowAndroid,
(focus, type)
-> setUrlBarFocus(focus, type),
requestFocusRunnable, shouldShowUpdateBadge, isInOverviewModeSupplier,
isCustomTab ? mCustomTabThemeColorProvider : browsingModeThemeColorProvider,
R.id.menu_button_wrapper);
MenuButtonCoordinator startSurfaceMenuButtonCoordinator = new MenuButtonCoordinator(
appMenuCoordinatorSupplier, mControlsVisibilityDelegate, mActivity,
appMenuCoordinatorSupplier, mControlsVisibilityDelegate, mWindowAndroid,
(focus, type)
-> setUrlBarFocus(focus, type),
requestFocusRunnable, shouldShowUpdateBadge, isInOverviewModeSupplier,
......
......@@ -26,6 +26,7 @@ import org.chromium.chrome.browser.ui.appmenu.AppMenuButtonHelper;
import org.chromium.chrome.browser.ui.appmenu.AppMenuCoordinator;
import org.chromium.ui.UiUtils;
import org.chromium.ui.base.ViewUtils;
import org.chromium.ui.base.WindowAndroid;
import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.modelutil.PropertyModelChangeProcessor;
......@@ -50,7 +51,7 @@ public class MenuButtonCoordinator {
* app menu MVC components.
* @param controlsVisibilityDelegate Delegate for forcing persistent display of browser
* controls.
* @param activity Activity in which this object lives.
* @param windowAndroid The WindowAndroid instance.
* @param setUrlBarFocusFunction Function that allows setting focus on the url bar.
* @param requestRenderRunnable Runnable that requests a re-rendering of the compositor view
* containing the app menu button.
......@@ -62,11 +63,11 @@ public class MenuButtonCoordinator {
*/
public MenuButtonCoordinator(OneshotSupplier<AppMenuCoordinator> appMenuCoordinatorSupplier,
BrowserStateBrowserControlsVisibilityDelegate controlsVisibilityDelegate,
Activity activity, SetFocusFunction setUrlBarFocusFunction,
WindowAndroid windowAndroid, SetFocusFunction setUrlBarFocusFunction,
Runnable requestRenderRunnable, boolean shouldShowAppUpdateBadge,
Supplier<Boolean> isInOverviewModeSupplier, ThemeColorProvider themeColorProvider,
@IdRes int menuButtonId) {
mActivity = activity;
mActivity = windowAndroid.getActivity().get();
mMenuButton = mActivity.findViewById(menuButtonId);
mPropertyModel = new PropertyModel.Builder(MenuButtonProperties.ALL_KEYS)
.with(MenuButtonProperties.SHOW_UPDATE_BADGE,
......@@ -81,7 +82,7 @@ public class MenuButtonCoordinator {
-> mActivity.isFinishing() || mActivity.isDestroyed(),
requestRenderRunnable, themeColorProvider, isInOverviewModeSupplier,
controlsVisibilityDelegate, setUrlBarFocusFunction, appMenuCoordinatorSupplier,
mActivity.getResources());
windowAndroid);
mMediator.getMenuButtonHelperSupplier().addObserver(
(helper) -> mAppMenuButtonHelper = helper);
if (mMenuButton != null) {
......
......@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.toolbar.menu_button;
import android.animation.Animator;
import android.animation.AnimatorSet;
import android.app.Activity;
import android.content.res.ColorStateList;
import android.content.res.Resources;
......@@ -32,6 +33,8 @@ import org.chromium.chrome.browser.ui.appmenu.AppMenuCoordinator;
import org.chromium.chrome.browser.ui.appmenu.AppMenuHandler;
import org.chromium.chrome.browser.ui.appmenu.AppMenuObserver;
import org.chromium.chrome.browser.ui.appmenu.AppMenuPropertiesDelegate;
import org.chromium.ui.KeyboardVisibilityDelegate;
import org.chromium.ui.base.WindowAndroid;
import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.modelutil.PropertyModelAnimatorFactory;
import org.chromium.ui.util.TokenHolder;
......@@ -51,6 +54,8 @@ class MenuButtonMediator implements AppMenuObserver {
private final PropertyModel mPropertyModel;
private final Runnable mRequestRenderRunnable;
private final ThemeColorProvider mThemeColorProvider;
private final Activity mActivity;
private final KeyboardVisibilityDelegate mKeyboardDelegate;
private boolean mShouldShowAppUpdateBadge;
private Runnable mUpdateStateChangedListener;
private Supplier<Boolean> mIsActivityFinishingSupplier;
......@@ -75,7 +80,7 @@ class MenuButtonMediator implements AppMenuObserver {
* a pending update.
* @param isInOverviewModeSupplier Supplier of overview mode state.
* @param themeColorProvider Provider of theme color changes.
* @param resources Resources object to use to obtain, e.g. localized strings.
* @param windowAndroid The WindowAndroid instance.
* @param shouldShowAppUpdateBadge Whether the "update available" badge should ever be shown.
* @param isActivityFinishingSupplier Supplier for knowing if the embedding activity is in the
* process of finishing or has already been destroyed.
......@@ -86,7 +91,8 @@ class MenuButtonMediator implements AppMenuObserver {
ThemeColorProvider themeColorProvider, Supplier<Boolean> isInOverviewModeSupplier,
BrowserStateBrowserControlsVisibilityDelegate controlsVisibilityDelegate,
SetFocusFunction setUrlBarFocusFunction,
OneshotSupplier<AppMenuCoordinator> appMenuCoordinatorSupplier, Resources resources) {
OneshotSupplier<AppMenuCoordinator> appMenuCoordinatorSupplier,
WindowAndroid windowAndroid) {
mPropertyModel = propertyModel;
mShouldShowAppUpdateBadge = shouldShowAppUpdateBadge;
mIsActivityFinishingSupplier = isActivityFinishingSupplier;
......@@ -99,8 +105,10 @@ class MenuButtonMediator implements AppMenuObserver {
mAppMenuCoordinatorSupplierObserver = this::onAppMenuInitialized;
mAppMenuCoordinatorSupplier = appMenuCoordinatorSupplier;
mAppMenuCoordinatorSupplier.onAvailable(mAppMenuCoordinatorSupplierObserver);
mResources = resources;
mActivity = windowAndroid.getActivity().get();
mResources = mActivity.getResources();
mAppMenuButtonHelperSupplier = new ObservableSupplierImpl<>();
mKeyboardDelegate = windowAndroid.getKeyboardDelegate();
mUrlFocusTranslationX =
mResources.getDimensionPixelSize(R.dimen.toolbar_url_focus_translation_x);
......@@ -112,6 +120,8 @@ class MenuButtonMediator implements AppMenuObserver {
// Defocus here to avoid handling focus in multiple places, e.g., when the
// forward button is pressed. (see crbug.com/414219)
mSetUrlBarFocusFunction.setFocus(false, OmniboxFocusReason.UNFOCUS);
// Dismiss keyboard in case the user was interacting with an input field on a website.
mKeyboardDelegate.hideKeyboard(mActivity.getCurrentFocus());
if (!mIsInOverviewModeSupplier.get() && isShowingAppMenuUpdateBadge()) {
// The app menu badge should be removed the first time the menu is opened.
......
......@@ -27,6 +27,10 @@ import org.chromium.chrome.browser.ui.appmenu.AppMenuButtonHelper;
import org.chromium.chrome.browser.ui.appmenu.AppMenuCoordinator;
import org.chromium.chrome.browser.ui.appmenu.AppMenuHandler;
import org.chromium.chrome.browser.ui.appmenu.AppMenuPropertiesDelegate;
import org.chromium.ui.KeyboardVisibilityDelegate;
import org.chromium.ui.base.WindowAndroid;
import java.lang.ref.WeakReference;
/**
* Unit tests for ToolbarAppMenuManager.
......@@ -59,6 +63,10 @@ public class MenuButtonCoordinatorTest {
ThemeColorProvider mThemeColorProvider;
@Mock
Resources mResources;
@Mock
private WindowAndroid mWindowAndroid;
@Mock
private KeyboardVisibilityDelegate mKeyboardDelegate;
private UpdateMenuItemHelper.MenuUiState mMenuUiState;
private OneshotSupplierImpl<AppMenuCoordinator> mAppMenuSupplier;
......@@ -84,9 +92,11 @@ public class MenuButtonCoordinatorTest {
doReturn(10)
.when(mResources)
.getDimensionPixelSize(org.chromium.chrome.R.dimen.toolbar_url_focus_translation_x);
doReturn(new WeakReference<>(mActivity)).when(mWindowAndroid).getActivity();
doReturn(mKeyboardDelegate).when(mWindowAndroid).getKeyboardDelegate();
mMenuButtonCoordinator = new MenuButtonCoordinator(mAppMenuSupplier,
mControlsVisibilityDelegate, mActivity, mFocusFunction, mRequestRenderRunnable,
mControlsVisibilityDelegate, mWindowAndroid, mFocusFunction, mRequestRenderRunnable,
true,
() -> false, mThemeColorProvider, org.chromium.chrome.R.id.menu_button_wrapper);
}
......
......@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.toolbar.menu_button;
import static junit.framework.Assert.assertFalse;
import static junit.framework.Assert.assertTrue;
import static org.mockito.Mockito.anyObject;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
......@@ -32,9 +33,13 @@ import org.chromium.chrome.browser.ui.appmenu.AppMenuButtonHelper;
import org.chromium.chrome.browser.ui.appmenu.AppMenuCoordinator;
import org.chromium.chrome.browser.ui.appmenu.AppMenuHandler;
import org.chromium.chrome.browser.ui.appmenu.AppMenuPropertiesDelegate;
import org.chromium.ui.KeyboardVisibilityDelegate;
import org.chromium.ui.base.WindowAndroid;
import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.util.TokenHolder;
import java.lang.ref.WeakReference;
/**
* Unit tests for ToolbarAppMenuManager.
*/
......@@ -62,6 +67,10 @@ public class MenuButtonMediatorTest {
ThemeColorProvider mThemeColorProvider;
@Mock
Resources mResources;
@Mock
private WindowAndroid mWindowAndroid;
@Mock
private KeyboardVisibilityDelegate mKeyboardDelegate;
private UpdateMenuItemHelper.MenuUiState mMenuUiState;
private OneshotSupplierImpl<AppMenuCoordinator> mAppMenuSupplier;
......@@ -88,6 +97,9 @@ public class MenuButtonMediatorTest {
mAppMenuSupplier = new OneshotSupplierImpl<>();
mMenuUiState = new UpdateMenuItemHelper.MenuUiState();
doReturn(mMenuUiState).when(mUpdateMenuItemHelper).getUiState();
doReturn(mResources).when(mActivity).getResources();
doReturn(new WeakReference<>(mActivity)).when(mWindowAndroid).getActivity();
doReturn(mKeyboardDelegate).when(mWindowAndroid).getKeyboardDelegate();
mMenuButtonMediator = new MenuButtonMediator(mPropertyModel, true,
()
......@@ -95,7 +107,7 @@ public class MenuButtonMediatorTest {
mRequestRenderRunnable, mThemeColorProvider,
()
-> false,
mControlsVisibilityDelegate, mFocusFunction, mAppMenuSupplier, mResources);
mControlsVisibilityDelegate, mFocusFunction, mAppMenuSupplier, mWindowAndroid);
}
@Test
......@@ -170,7 +182,7 @@ public class MenuButtonMediatorTest {
mRequestRenderRunnable, mThemeColorProvider,
()
-> false,
mControlsVisibilityDelegate, mFocusFunction, mAppMenuSupplier, mResources);
mControlsVisibilityDelegate, mFocusFunction, mAppMenuSupplier, mWindowAndroid);
doReturn(true).when(mActivity).isDestroyed();
newMediator.updateStateChanged();
......@@ -202,4 +214,10 @@ public class MenuButtonMediatorTest {
mMenuButtonMediator.updateReloadingState(true);
mMenuButtonMediator.updateStateChanged();
}
@Test
public void testKeyboardIsDismissedWhenMenuShows() {
mMenuButtonMediator.onMenuVisibilityChanged(true);
verify(mKeyboardDelegate).hideKeyboard(anyObject());
}
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment