Show “find in page” menu item on PDF native page
No need to update Custom Tab, since “find in page” is always shown for CCT except for CustomTabsUiType.MEDIA_VIEWER. Will override “find in page” behavior (invoke pdf viewer API) in a separate CL. Bug: 335740993 Change-Id: Ibd72a3bc05475e1d67c78d8d36de252ed76a0af9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5479831 Reviewed-by: Theresa Sullivan <twellington@chromium.org> Reviewed-by: Michael Thiessen <mthiesse@chromium.org> Commit-Queue: Michael Thiessen <mthiesse@chromium.org> Auto-Submit: Shu Yang <shuyng@google.com> Cr-Commit-Position: refs/heads/main@{#1291861}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
c3321dc8bc
commit
8b6aca02b5
chrome/android
java
src
org
chromium
chrome
browser
app
junit
src
org
chromium
chrome
browser
app
url/android/test/java/src/org/chromium/url
27
chrome/android/java/src/org/chromium/chrome/browser/app/appmenu/AppMenuPropertiesDelegateImpl.java
27
chrome/android/java/src/org/chromium/chrome/browser/app/appmenu/AppMenuPropertiesDelegateImpl.java
@ -553,10 +553,8 @@ public class AppMenuPropertiesDelegateImpl implements AppMenuPropertiesDelegate
|
||||
&& shouldShowWebContentsDependentMenuItem(currentTab)
|
||||
&& PageZoomCoordinator.shouldShowMenuItem());
|
||||
|
||||
// Disable find in page on the native NTP or on Start surface.
|
||||
menu.findItem(R.id.find_in_page_id)
|
||||
.setVisible(
|
||||
isCurrentTabNotNull && shouldShowWebContentsDependentMenuItem(currentTab));
|
||||
// Disable find in page on the native NTP (except for PDF native page) or on Start surface.
|
||||
updateFindInPageMenuItem(menu, currentTab);
|
||||
|
||||
// Prepare translate menu button.
|
||||
prepareTranslateMenuItem(menu, currentTab);
|
||||
@ -821,8 +819,8 @@ public class AppMenuPropertiesDelegateImpl implements AppMenuPropertiesDelegate
|
||||
|
||||
/**
|
||||
* @param currentTab The currentTab for which the app menu is showing.
|
||||
* @return Whether the currentTab should show an app menu item that requires a webContents.
|
||||
* This will return false for the Start service or native NTP, and true otherwise.
|
||||
* @return Whether the currentTab should show an app menu item that requires a webContents. This
|
||||
* will return false for the Start surface or native NTP, and true otherwise.
|
||||
*/
|
||||
protected boolean shouldShowWebContentsDependentMenuItem(@NonNull Tab currentTab) {
|
||||
return !currentTab.isNativePage() && currentTab.getWebContents() != null;
|
||||
@ -1257,6 +1255,23 @@ public class AppMenuPropertiesDelegateImpl implements AppMenuPropertiesDelegate
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Updates the find in page menu item's state.
|
||||
*
|
||||
* @param menu {@link Menu} for find in page.
|
||||
* @param currentTab Current tab being displayed.
|
||||
*/
|
||||
private void updateFindInPageMenuItem(Menu menu, @Nullable Tab currentTab) {
|
||||
MenuItem findInPageMenuRow = menu.findItem(R.id.find_in_page_id);
|
||||
// PDF native page should show find in page menu item.
|
||||
boolean itemVisible =
|
||||
currentTab != null
|
||||
&& (shouldShowWebContentsDependentMenuItem(currentTab)
|
||||
|| (currentTab.isNativePage()
|
||||
&& currentTab.getNativePage().isPdf()));
|
||||
findInPageMenuRow.setVisible(itemVisible);
|
||||
}
|
||||
|
||||
/**
|
||||
* Updates the request desktop site item's state.
|
||||
*
|
||||
|
@ -93,6 +93,7 @@ import org.chromium.chrome.browser.ui.appmenu.AppMenuHandler;
|
||||
import org.chromium.chrome.browser.ui.appmenu.AppMenuItemProperties;
|
||||
import org.chromium.chrome.browser.ui.appmenu.AppMenuPropertiesDelegate;
|
||||
import org.chromium.chrome.browser.ui.appmenu.CustomViewBinder;
|
||||
import org.chromium.chrome.browser.ui.native_page.NativePage;
|
||||
import org.chromium.chrome.browser.webapps.WebappRegistry;
|
||||
import org.chromium.components.bookmarks.BookmarkId;
|
||||
import org.chromium.components.browser_ui.accessibility.PageZoomCoordinator;
|
||||
@ -160,6 +161,7 @@ public class AppMenuPropertiesDelegateUnitTest {
|
||||
@Mock private TranslateBridge.Natives mTranslateBridgeJniMock;
|
||||
@Mock private AppMenuHandler mAppMenuHandler;
|
||||
@Mock private AppMenuPropertiesDelegate.CustomItemViewTypeProvider mCustomItemViewTypeProvider;
|
||||
@Mock private NativePage mNativePage;
|
||||
private OneshotSupplierImpl<IncognitoReauthController> mIncognitoReauthControllerSupplier =
|
||||
new OneshotSupplierImpl<>();
|
||||
private OneshotSupplierImpl<LayoutStateProvider> mLayoutStateProviderSupplier =
|
||||
@ -329,6 +331,8 @@ public class AppMenuPropertiesDelegateUnitTest {
|
||||
setUpMocksForPageMenu();
|
||||
when(mTab.getUrl()).thenReturn(JUnitTestGURLs.NTP_URL);
|
||||
when(mTab.isNativePage()).thenReturn(true);
|
||||
when(mNativePage.isPdf()).thenReturn(false);
|
||||
when(mTab.getNativePage()).thenReturn(mNativePage);
|
||||
doReturn(false)
|
||||
.when(mAppMenuPropertiesDelegate)
|
||||
.shouldShowTranslateMenuItem(any(Tab.class));
|
||||
@ -355,6 +359,43 @@ public class AppMenuPropertiesDelegateUnitTest {
|
||||
assertMenuItemsAreEqual(menu, expectedItems);
|
||||
}
|
||||
|
||||
@Test
|
||||
@Config(qualifiers = "sw320dp")
|
||||
public void testPageMenuItems_Phone_Pdf() {
|
||||
setUpMocksForPageMenu();
|
||||
when(mTab.getUrl()).thenReturn(JUnitTestGURLs.URL_1_WITH_PDF_PATH);
|
||||
when(mTab.isNativePage()).thenReturn(true);
|
||||
when(mNativePage.isPdf()).thenReturn(true);
|
||||
when(mTab.getNativePage()).thenReturn(mNativePage);
|
||||
doReturn(false)
|
||||
.when(mAppMenuPropertiesDelegate)
|
||||
.shouldShowTranslateMenuItem(any(Tab.class));
|
||||
|
||||
Assert.assertEquals(MenuGroup.PAGE_MENU, mAppMenuPropertiesDelegate.getMenuGroup());
|
||||
Menu menu = createTestMenu();
|
||||
mAppMenuPropertiesDelegate.prepareMenu(menu, null);
|
||||
|
||||
Integer[] expectedItems = {
|
||||
R.id.icon_row_menu_id,
|
||||
R.id.new_tab_menu_id,
|
||||
R.id.new_incognito_tab_menu_id,
|
||||
R.id.divider_line_id,
|
||||
R.id.open_history_menu_id,
|
||||
R.id.quick_delete_menu_id,
|
||||
R.id.quick_delete_divider_line_id,
|
||||
R.id.downloads_menu_id,
|
||||
R.id.all_bookmarks_menu_id,
|
||||
R.id.recent_tabs_menu_id,
|
||||
R.id.divider_line_id,
|
||||
R.id.share_row_menu_id,
|
||||
R.id.find_in_page_id,
|
||||
R.id.divider_line_id,
|
||||
R.id.preferences_id,
|
||||
R.id.help_id
|
||||
};
|
||||
assertMenuItemsAreEqual(menu, expectedItems);
|
||||
}
|
||||
|
||||
@Test
|
||||
@Config(qualifiers = "sw320dp")
|
||||
public void testPageMenuItems_Phone_RegularPage() {
|
||||
|
@ -10,6 +10,7 @@ public class JUnitTestGURLs {
|
||||
public static final GURL HTTP_URL = new GURL("http://www.example.com/");
|
||||
public static final GURL URL_1 = new GURL("https://www.one.com/");
|
||||
public static final GURL URL_1_WITH_PATH = new GURL("https://www.one.com/some_path.html");
|
||||
public static final GURL URL_1_WITH_PDF_PATH = new GURL("https://www.one.com/some_path.pdf");
|
||||
public static final GURL URL_2 = new GURL("https://www.two.com/");
|
||||
public static final GURL URL_3 = new GURL("https://www.three.com/");
|
||||
public static final GURL MAPS_URL = new GURL("https://maps.google.com/");
|
||||
|
Reference in New Issue
Block a user