From b6ec1be4922043b0f8031f542f1e440d5c2f08bb Mon Sep 17 00:00:00 2001 From: Vladislav Kaznacheev Date: Wed, 31 Jan 2018 16:13:58 -0800 Subject: [PATCH] [DO NOT MERGE] Fix context menu position for RTL Based on https://android-review.googlesource.com/574843. Added APCT coverage to verify the fix and prevent regressions. Bug: 70920189 Test: android.view.menu.ContextMenuTest Change-Id: Id9ee500751fe6f3da07bf10fb510ac49487104d0 --- .../internal/view/menu/MenuPopupHelper.java | 2 +- .../internal/view/menu/StandardMenuPopup.java | 12 +- core/tests/coretests/AndroidManifest.xml | 7 ++ .../coretests/res/layout/context_menu.xml | 54 +++++++++ .../src/android/util/PollingCheck.java | 104 ++++++++++++++++++ .../view/menu/ContextMenuActivity.java | 54 +++++++++ .../android/view/menu/ContextMenuTest.java | 93 ++++++++++++++++ .../widget/espresso/ContextMenuUtils.java | 90 ++++++++++++++- 8 files changed, 407 insertions(+), 9 deletions(-) create mode 100644 core/tests/coretests/res/layout/context_menu.xml create mode 100644 core/tests/coretests/src/android/util/PollingCheck.java create mode 100644 core/tests/coretests/src/android/view/menu/ContextMenuActivity.java create mode 100644 core/tests/coretests/src/android/view/menu/ContextMenuTest.java diff --git a/core/java/com/android/internal/view/menu/MenuPopupHelper.java b/core/java/com/android/internal/view/menu/MenuPopupHelper.java index 6af41a51f0ddb..324f923674eb1 100644 --- a/core/java/com/android/internal/view/menu/MenuPopupHelper.java +++ b/core/java/com/android/internal/view/menu/MenuPopupHelper.java @@ -256,7 +256,7 @@ public class MenuPopupHelper implements MenuHelper { final int hgrav = Gravity.getAbsoluteGravity(mDropDownGravity, mAnchorView.getLayoutDirection()) & Gravity.HORIZONTAL_GRAVITY_MASK; if (hgrav == Gravity.RIGHT) { - xOffset += mAnchorView.getWidth(); + xOffset -= mAnchorView.getWidth(); } popup.setHorizontalOffset(xOffset); diff --git a/core/java/com/android/internal/view/menu/StandardMenuPopup.java b/core/java/com/android/internal/view/menu/StandardMenuPopup.java index d9ca5be0502eb..445379b1d9f49 100644 --- a/core/java/com/android/internal/view/menu/StandardMenuPopup.java +++ b/core/java/com/android/internal/view/menu/StandardMenuPopup.java @@ -263,7 +263,6 @@ final class StandardMenuPopup extends MenuPopup implements OnDismissListener, On mShownAnchorView, mOverflowOnly, mPopupStyleAttr, mPopupStyleRes); subPopup.setPresenterCallback(mPresenterCallback); subPopup.setForceShowIcon(MenuPopup.shouldPreserveIconSpacing(subMenu)); - subPopup.setGravity(mDropDownGravity); // Pass responsibility for handling onDismiss to the submenu. subPopup.setOnDismissListener(mOnDismissListener); @@ -273,8 +272,17 @@ final class StandardMenuPopup extends MenuPopup implements OnDismissListener, On mMenu.close(false /* closeAllMenus */); // Show the new sub-menu popup at the same location as this popup. - final int horizontalOffset = mPopup.getHorizontalOffset(); + int horizontalOffset = mPopup.getHorizontalOffset(); final int verticalOffset = mPopup.getVerticalOffset(); + + // As xOffset of parent menu popup is subtracted with Anchor width for Gravity.RIGHT, + // So, again to display sub-menu popup in same xOffset, add the Anchor width. + final int hgrav = Gravity.getAbsoluteGravity(mDropDownGravity, + mAnchorView.getLayoutDirection()) & Gravity.HORIZONTAL_GRAVITY_MASK; + if (hgrav == Gravity.RIGHT) { + horizontalOffset += mAnchorView.getWidth(); + } + if (subPopup.tryShow(horizontalOffset, verticalOffset)) { if (mPresenterCallback != null) { mPresenterCallback.onOpenSubMenu(subMenu); diff --git a/core/tests/coretests/AndroidManifest.xml b/core/tests/coretests/AndroidManifest.xml index ab9912a438d4c..c0a8acda628ed 100644 --- a/core/tests/coretests/AndroidManifest.xml +++ b/core/tests/coretests/AndroidManifest.xml @@ -1041,6 +1041,13 @@ + + + + + + + diff --git a/core/tests/coretests/res/layout/context_menu.xml b/core/tests/coretests/res/layout/context_menu.xml new file mode 100644 index 0000000000000..3b9e2bdb3130c --- /dev/null +++ b/core/tests/coretests/res/layout/context_menu.xml @@ -0,0 +1,54 @@ + + + + + + + + + + + + + + diff --git a/core/tests/coretests/src/android/util/PollingCheck.java b/core/tests/coretests/src/android/util/PollingCheck.java new file mode 100644 index 0000000000000..468b9b2a4864d --- /dev/null +++ b/core/tests/coretests/src/android/util/PollingCheck.java @@ -0,0 +1,104 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.util; + +import org.junit.Assert; + +/** + * Utility used for testing that allows to poll for a certain condition to happen within a timeout. + * + * Code copied from com.android.compatibility.common.util.PollingCheck + */ +public abstract class PollingCheck { + + private static final long DEFAULT_TIMEOUT = 3000; + private static final long TIME_SLICE = 50; + private final long mTimeout; + + /** + * The condition that the PollingCheck should use to proceed successfully. + */ + public interface PollingCheckCondition { + + /** + * @return Whether the polling condition has been met. + */ + boolean canProceed(); + } + + public PollingCheck(long timeout) { + mTimeout = timeout; + } + + protected abstract boolean check(); + + /** + * Start running the polling check. + */ + public void run() { + if (check()) { + return; + } + + long timeout = mTimeout; + while (timeout > 0) { + try { + Thread.sleep(TIME_SLICE); + } catch (InterruptedException e) { + Assert.fail("unexpected InterruptedException"); + } + + if (check()) { + return; + } + + timeout -= TIME_SLICE; + } + + Assert.fail("unexpected timeout"); + } + + /** + * Instantiate and start polling for a given condition with a default 3000ms timeout. + * + * @param condition The condition to check for success. + */ + public static void waitFor(final PollingCheckCondition condition) { + new PollingCheck(DEFAULT_TIMEOUT) { + @Override + protected boolean check() { + return condition.canProceed(); + } + }.run(); + } + + /** + * Instantiate and start polling for a given condition. + * + * @param timeout Time out in ms + * @param condition The condition to check for success. + */ + public static void waitFor(long timeout, final PollingCheckCondition condition) { + new PollingCheck(timeout) { + @Override + protected boolean check() { + return condition.canProceed(); + } + }.run(); + } +} + diff --git a/core/tests/coretests/src/android/view/menu/ContextMenuActivity.java b/core/tests/coretests/src/android/view/menu/ContextMenuActivity.java new file mode 100644 index 0000000000000..830b3d549773f --- /dev/null +++ b/core/tests/coretests/src/android/view/menu/ContextMenuActivity.java @@ -0,0 +1,54 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.view.menu; + +import android.app.Activity; +import android.os.Bundle; +import android.view.ContextMenu; +import android.view.ContextMenu.ContextMenuInfo; +import android.view.View; + +import com.android.frameworks.coretests.R; + +public class ContextMenuActivity extends Activity { + + static final String LABEL_ITEM = "Item"; + static final String LABEL_SUBMENU = "Submenu"; + static final String LABEL_SUBITEM = "Subitem"; + + @Override + protected void onCreate(Bundle savedInstanceState) { + super.onCreate(savedInstanceState); + setContentView(R.layout.context_menu); + registerForContextMenu(getTargetLtr()); + registerForContextMenu(getTargetRtl()); + } + + @Override + public void onCreateContextMenu(ContextMenu menu, View v, ContextMenuInfo menuInfo) { + menu.add(LABEL_ITEM); + menu.addSubMenu(LABEL_SUBMENU).add(LABEL_SUBITEM); + } + + View getTargetLtr() { + return findViewById(R.id.context_menu_target_ltr); + } + + View getTargetRtl() { + return findViewById(R.id.context_menu_target_rtl); + } +} diff --git a/core/tests/coretests/src/android/view/menu/ContextMenuTest.java b/core/tests/coretests/src/android/view/menu/ContextMenuTest.java new file mode 100644 index 0000000000000..59d4e55d8d45f --- /dev/null +++ b/core/tests/coretests/src/android/view/menu/ContextMenuTest.java @@ -0,0 +1,93 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.view.menu; + +import android.content.Context; +import android.graphics.Point; +import android.support.test.filters.MediumTest; +import android.test.ActivityInstrumentationTestCase; +import android.util.PollingCheck; +import android.view.Display; +import android.view.View; +import android.view.WindowManager; +import android.widget.espresso.ContextMenuUtils; + +@MediumTest +public class ContextMenuTest extends ActivityInstrumentationTestCase { + + public ContextMenuTest() { + super("com.android.frameworks.coretests", ContextMenuActivity.class); + } + + public void testContextMenuPositionLtr() throws InterruptedException { + testMenuPosition(getActivity().getTargetLtr()); + } + + public void testContextMenuPositionRtl() throws InterruptedException { + testMenuPosition(getActivity().getTargetRtl()); + } + + private void testMenuPosition(View target) throws InterruptedException { + final int minScreenDimension = getMinScreenDimension(); + if (minScreenDimension < 320) { + // Assume there is insufficient room for the context menu to be aligned properly. + return; + } + + int offsetX = target.getWidth() / 2; + int offsetY = target.getHeight() / 2; + + getInstrumentation().runOnMainSync(() -> target.performLongClick(offsetX, offsetY)); + + PollingCheck.waitFor( + () -> ContextMenuUtils.isMenuItemClickable(ContextMenuActivity.LABEL_SUBMENU)); + + ContextMenuUtils.assertContextMenuAlignment(target, offsetX, offsetY); + + ContextMenuUtils.clickMenuItem(ContextMenuActivity.LABEL_SUBMENU); + + PollingCheck.waitFor( + () -> ContextMenuUtils.isMenuItemClickable(ContextMenuActivity.LABEL_SUBITEM)); + + if (minScreenDimension < getCascadingMenuTreshold()) { + // A non-cascading submenu should be displayed at the same location as its parent. + // Not testing cascading submenu position, as it is positioned differently. + ContextMenuUtils.assertContextMenuAlignment(target, offsetX, offsetY); + } + } + + /** + * Returns the minimum of the default display's width and height. + */ + private int getMinScreenDimension() { + final WindowManager windowManager = (WindowManager) getActivity().getSystemService( + Context.WINDOW_SERVICE); + final Display display = windowManager.getDefaultDisplay(); + final Point displaySize = new Point(); + display.getRealSize(displaySize); + return Math.min(displaySize.x, displaySize.y); + } + + /** + * Returns the minimum display size where cascading submenus are supported. + */ + private int getCascadingMenuTreshold() { + // Use the same dimension resource as in MenuPopupHelper.createPopup(). + return getActivity().getResources().getDimensionPixelSize( + com.android.internal.R.dimen.cascading_menus_min_smallest_width); + } +} diff --git a/core/tests/coretests/src/android/widget/espresso/ContextMenuUtils.java b/core/tests/coretests/src/android/widget/espresso/ContextMenuUtils.java index c8218aa490f26..487a881082e73 100644 --- a/core/tests/coretests/src/android/widget/espresso/ContextMenuUtils.java +++ b/core/tests/coretests/src/android/widget/espresso/ContextMenuUtils.java @@ -17,25 +17,32 @@ package android.widget.espresso; import static android.support.test.espresso.Espresso.onView; +import static android.support.test.espresso.action.ViewActions.click; import static android.support.test.espresso.assertion.ViewAssertions.matches; import static android.support.test.espresso.matcher.RootMatchers.withDecorView; import static android.support.test.espresso.matcher.ViewMatchers.hasDescendant; import static android.support.test.espresso.matcher.ViewMatchers.hasFocus; import static android.support.test.espresso.matcher.ViewMatchers.isAssignableFrom; import static android.support.test.espresso.matcher.ViewMatchers.isDisplayed; +import static android.support.test.espresso.matcher.ViewMatchers.isDisplayingAtLeast; import static android.support.test.espresso.matcher.ViewMatchers.isEnabled; import static android.support.test.espresso.matcher.ViewMatchers.withText; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.not; -import com.android.internal.view.menu.ListMenuItemView; - import android.support.test.espresso.NoMatchingRootException; import android.support.test.espresso.NoMatchingViewException; import android.support.test.espresso.ViewInteraction; import android.support.test.espresso.matcher.ViewMatchers; +import android.view.View; import android.widget.MenuPopupWindow.MenuDropDownListView; +import com.android.internal.view.menu.ListMenuItemView; + +import org.hamcrest.Description; +import org.hamcrest.Matcher; +import org.hamcrest.TypeSafeMatcher; + /** * Espresso utility methods for the context menu. */ @@ -82,10 +89,15 @@ public final class ContextMenuUtils { private static void asssertContextMenuContainsItemWithEnabledState(String itemLabel, boolean enabled) { onContextMenu().check(matches( - hasDescendant(allOf( - isAssignableFrom(ListMenuItemView.class), - enabled ? isEnabled() : not(isEnabled()), - hasDescendant(withText(itemLabel)))))); + hasDescendant(getVisibleMenuItemMatcher(itemLabel, enabled)))); + } + + private static Matcher getVisibleMenuItemMatcher(String itemLabel, boolean enabled) { + return allOf( + isAssignableFrom(ListMenuItemView.class), + hasDescendant(withText(itemLabel)), + enabled ? isEnabled() : not(isEnabled()), + isDisplayingAtLeast(90)); } /** @@ -107,4 +119,70 @@ public final class ContextMenuUtils { public static void assertContextMenuContainsItemDisabled(String itemLabel) { asssertContextMenuContainsItemWithEnabledState(itemLabel, false); } + + /** + * Asserts that the context menu window is aligned to a given view with a given offset. + * + * @param anchor Anchor view. + * @param offsetX x offset + * @param offsetY y offset. + * @throws AssertionError if the assertion fails + */ + public static void assertContextMenuAlignment(View anchor, int offsetX, int offsetY) { + int [] expectedLocation = new int[2]; + anchor.getLocationOnScreen(expectedLocation); + expectedLocation[0] += offsetX; + expectedLocation[1] += offsetY; + + final boolean rtl = anchor.getLayoutDirection() == View.LAYOUT_DIRECTION_RTL; + + onContextMenu().check(matches(new TypeSafeMatcher() { + @Override + public void describeTo(Description description) { + description.appendText("root view "); + description.appendText(rtl ? "right" : "left"); + description.appendText("="); + description.appendText(Integer.toString(offsetX)); + description.appendText(", top="); + description.appendText(Integer.toString(offsetY)); + } + + @Override + public boolean matchesSafely(View view) { + View rootView = view.getRootView(); + int [] actualLocation = new int[2]; + rootView.getLocationOnScreen(actualLocation); + if (rtl) { + actualLocation[0] += rootView.getWidth(); + } + return expectedLocation[0] == actualLocation[0] + && expectedLocation[1] == actualLocation[1]; + } + })); + } + + /** + * Check is the menu item is clickable (i.e. visible and enabled). + * + * @param itemLabel Label of the item. + * @return True if the menu item is clickable. + */ + public static boolean isMenuItemClickable(String itemLabel) { + try { + onContextMenu().check(matches( + hasDescendant(getVisibleMenuItemMatcher(itemLabel, true)))); + return true; + } catch (NoMatchingRootException | NoMatchingViewException | AssertionError e) { + return false; + } + } + + /** + * Click on a menu item with the specified label + * @param itemLabel Label of the item. + */ + public static void clickMenuItem(String itemLabel) { + onView(getVisibleMenuItemMatcher(itemLabel, true)) + .inRoot(withDecorView(hasFocus())).perform(click()); + } }