[context menu] add page save option to empty space CM
Screen record:
https://screencast.googleplex.com/cast/NTc4MTc5OTAwMjE3NzUzNnwwNDE5MmJkMC00ZA
Note: Only IDS_CONTEXTMENU_SAVE_PAGE is an UI string and the page group
title is not used anywhere, hence skipping the translation screenshots
check.
Bug: 391719844
Skip-Translation-Screenshots-Check: True
Test: tools/autotest.py -C out/Default ChromeContextMenuPopulatorTest
Change-Id: If047f6250f90092fe5bae8e25850adf020c92ba3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6333204
Reviewed-by: David Trainor <dtrainor@chromium.org>
Auto-Submit: Grace Cham <hscham@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Commit-Queue: Grace Cham <hscham@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1437290}
diff --git a/chrome/android/java/res/values/ids.xml b/chrome/android/java/res/values/ids.xml
index 05457c5..65d44f7b 100644
--- a/chrome/android/java/res/values/ids.xml
+++ b/chrome/android/java/res/values/ids.xml
@@ -42,6 +42,9 @@
<item type="id" name="password_infobar_accounts_spinner" />
<!-- Context Menu Ids -->
+ <!-- Page Group -->
+ <item type="id" name="contextmenu_save_page" />
+
<!-- Custom Tab Group -->
<item type="id" name="contextmenu_open_in_new_chrome_tab" />
<item type="id" name="contextmenu_open_in_chrome_incognito_tab" />
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuItem.java b/chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuItem.java
index 8051698..1d672fd 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuItem.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuItem.java
@@ -63,7 +63,8 @@
Item.OPEN_IN_NEW_TAB_IN_GROUP,
Item.SHARE_HIGHLIGHT,
Item.REMOVE_HIGHLIGHT,
- Item.LEARN_MORE
+ Item.LEARN_MORE,
+ Item.SAVE_PAGE,
})
@Retention(RetentionPolicy.SOURCE)
public @interface Item {
@@ -111,8 +112,10 @@
int SHARE_HIGHLIGHT = 32;
int REMOVE_HIGHLIGHT = 33;
int LEARN_MORE = 34;
+ // Page Group
+ int SAVE_PAGE = 35;
// ALWAYS UPDATE!
- int NUM_ENTRIES = 35;
+ int NUM_ENTRIES = 36;
}
/** Mapping from {@link Item} to the ID found in the ids.xml. */
@@ -152,6 +155,7 @@
R.id.contextmenu_share_highlight, // Item.SHARE_HIGHLIGHT
R.id.contextmenu_remove_highlight, // Item.REMOVE_HIGHLIGHT
R.id.contextmenu_learn_more, // Item.LEARN_MORE
+ R.id.contextmenu_save_page, // Item.SAVE_PAGE
};
/** Mapping from {@link Item} to the ID of the string that describes the action of the item. */
@@ -191,6 +195,7 @@
R.string.contextmenu_share_highlight, // Item.SHARE_HIGHLIGHT
R.string.contextmenu_remove_highlight, // Item.REMOVE_HIGHLIGHT
R.string.contextmenu_learn_more, // Item.LEARN_MORE
+ R.string.contextmenu_save_page, // Item.SAVE_PAGE
};
/**
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java b/chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java
index 4168ffd..69c111f 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java
@@ -173,7 +173,8 @@
Action.SHARE_HIGHLIGHT,
Action.REMOVE_HIGHLIGHT,
Action.LEARN_MORE,
- Action.OPEN_IN_NEW_TAB_IN_GROUP
+ Action.OPEN_IN_NEW_TAB_IN_GROUP,
+ Action.SAVE_PAGE,
})
@Retention(RetentionPolicy.SOURCE)
public @interface Action {
@@ -218,7 +219,8 @@
int LEARN_MORE = 38;
int OPEN_IN_NEW_TAB_IN_GROUP = 39;
int OPEN_IN_NEW_WINDOW = 40;
- int NUM_ENTRIES = 41;
+ int SAVE_PAGE = 41;
+ int NUM_ENTRIES = 42;
}
}
@@ -275,7 +277,8 @@
return DeviceFormFactor.isNonMultiDisplayContextOnTablet(mContext);
}
- public static boolean shouldShowEmptySpaceContextMenu() {
+ @VisibleForTesting
+ boolean shouldShowEmptySpaceContextMenu() {
return DeviceFormFactor.isDesktop()
&& DeviceInput.supportsAlphabeticKeyboard()
&& DeviceInput.supportsPrecisionPointer();
@@ -288,7 +291,14 @@
List<Pair<Integer, ModelList>> groupedItems = new ArrayList<>();
if (mParams.isPage() && shouldShowEmptySpaceContextMenu()) {
- // TODO (crbug.com/391719844): add new groups and items for page actions.
+ ModelList pageGroup = new ModelList();
+ // TODO(crbug.com/405842034): investigate supporting downloads in incognito mode.
+ if (!mItemDelegate.isIncognito()
+ && UrlUtilities.isDownloadableScheme(mParams.getPageUrl())) {
+ pageGroup.add(
+ createListItem(Item.SAVE_PAGE, false, !mIsDownloadRestrictedByPolicy));
+ }
+ groupedItems.add(new Pair<>(R.string.contextmenu_page_title, pageGroup));
}
if (mParams.isAnchor()) {
@@ -616,6 +626,14 @@
} else if (mItemDelegate.startDownload(url, true)) {
mNativeDelegate.startDownload(url, false);
}
+ } else if (itemId == R.id.contextmenu_save_page) {
+ recordContextMenuSelection(ContextMenuUma.Action.SAVE_PAGE);
+ GURL url = mItemDelegate.getPageUrl();
+ if (mIsDownloadRestrictedByPolicy) {
+ showDownloadRestrictedToast();
+ } else if (mItemDelegate.startDownload(url, true)) {
+ mNativeDelegate.startDownload(url, false);
+ }
} else if (itemId == R.id.contextmenu_share_link) {
recordContextMenuSelection(ContextMenuUma.Action.SHARE_LINK);
// TODO(crbug.com/40549331): Migrate ShareParams to GURL.
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulatorTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulatorTest.java
index 0f03a5a..b91246e18 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulatorTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulatorTest.java
@@ -5,7 +5,6 @@
package org.chromium.chrome.browser.contextmenu;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.eq;
@@ -146,6 +145,12 @@
doReturn(false).when(mPopulator).shouldTriggerEphemeralTabHelpUi();
doReturn(false).when(mPopulator).shouldTriggerReadLaterHelpUi();
doReturn(true).when(mExternalAuthUtils).isGoogleSigned(IntentHandler.PACKAGE_GSA);
+ doReturn(false).when(mPopulator).shouldShowEmptySpaceContextMenu();
+ }
+
+ private void initializePopulatorOnDesktop(@ContextMenuMode int mode, ContextMenuParams params) {
+ initializePopulator(mode, params);
+ doReturn(true).when(mPopulator).shouldShowEmptySpaceContextMenu();
}
private void checkMenuOptions(List<Integer> disabled, int[]... groups) {
@@ -160,11 +165,10 @@
int[] availableInTab = new int[contextMenuState.get(i).second.size()];
for (int j = 0; j < contextMenuState.get(i).second.size(); j++) {
PropertyModel model = contextMenuState.get(i).second.get(j).model;
- if (disabled.contains(model.get(MENU_ID))) {
- assertFalse(model.get(ENABLED));
- } else {
- assertTrue(model.get(ENABLED));
- }
+ assertEquals(
+ "'" + model.get(TEXT) + "' has different enablement setting than expected",
+ !disabled.contains(model.get(MENU_ID)),
+ model.get(ENABLED));
availableInTab[j] = model.get(MENU_ID);
}
@@ -187,13 +191,25 @@
}
if (!Arrays.equals(expectedItemsInGroup, availableInTab)) {
- StringBuilder info = new StringBuilder();
+ StringBuilder generated_info = new StringBuilder();
for (int j = 0; j < contextMenuState.get(i).second.size(); j++) {
- info.append("'");
- info.append(contextMenuState.get(i).second.get(j).model.get(TEXT));
- info.append("' ");
+ generated_info.append("'");
+ generated_info.append(contextMenuState.get(i).second.get(j).model.get(TEXT));
+ generated_info.append("' ");
}
- fail("Items in group " + i + " don't match, generated: " + info.toString());
+ StringBuilder expected_info = new StringBuilder();
+ for (int j = 0; j < expectedItemsInGroup.length; j++) {
+ expected_info.append("'");
+ expected_info.append(expectedItemsInGroup[j]);
+ expected_info.append("' ");
+ }
+ fail(
+ "Items in group "
+ + i
+ + " don't match, expecting: "
+ + expected_info.toString()
+ + ", generated: "
+ + generated_info.toString());
}
}
}
@@ -1448,4 +1464,125 @@
};
checkMenuOptions(image2Expected);
}
+
+ @Test
+ @SmallTest
+ @UiThreadTest
+ public void testPage() {
+ FirstRunStatus.setFirstRunFlowComplete(true);
+ ContextMenuParams params =
+ new ContextMenuParams(
+ 0,
+ ContextMenuDataMediaType.NONE,
+ new GURL(PAGE_URL),
+ GURL.emptyGURL(),
+ "",
+ GURL.emptyGURL(),
+ GURL.emptyGURL(),
+ "",
+ null,
+ false,
+ 0,
+ 0,
+ MenuSourceType.TOUCH,
+ false,
+ false,
+ /* additionalNavigationParams= */ null);
+
+ int[] expected = {R.id.contextmenu_save_page};
+
+ initializePopulatorOnDesktop(ChromeContextMenuPopulator.ContextMenuMode.NORMAL, params);
+ checkMenuOptions(expected);
+
+ initializePopulatorOnDesktop(ChromeContextMenuPopulator.ContextMenuMode.CUSTOM_TAB, params);
+ checkMenuOptions(expected);
+
+ initializePopulatorOnDesktop(ChromeContextMenuPopulator.ContextMenuMode.WEB_APP, params);
+ checkMenuOptions(expected);
+
+ initializePopulatorOnDesktop(
+ ChromeContextMenuPopulator.ContextMenuMode.NETWORK_BOUND_TAB, params);
+ checkMenuOptions(expected);
+ }
+
+ @Test
+ @SmallTest
+ @UiThreadTest
+ public void testPageNotOnDesktop() {
+ FirstRunStatus.setFirstRunFlowComplete(true);
+ ContextMenuParams params =
+ new ContextMenuParams(
+ 0,
+ ContextMenuDataMediaType.NONE,
+ new GURL(PAGE_URL),
+ GURL.emptyGURL(),
+ "",
+ GURL.emptyGURL(),
+ GURL.emptyGURL(),
+ "",
+ null,
+ false,
+ 0,
+ 0,
+ MenuSourceType.TOUCH,
+ false,
+ false,
+ /* additionalNavigationParams= */ null);
+
+ int[] expected = null;
+
+ initializePopulator(ChromeContextMenuPopulator.ContextMenuMode.NORMAL, params);
+ checkMenuOptions(expected);
+
+ initializePopulator(ChromeContextMenuPopulator.ContextMenuMode.CUSTOM_TAB, params);
+ checkMenuOptions(expected);
+
+ initializePopulator(ChromeContextMenuPopulator.ContextMenuMode.WEB_APP, params);
+ checkMenuOptions(expected);
+
+ initializePopulator(ChromeContextMenuPopulator.ContextMenuMode.NETWORK_BOUND_TAB, params);
+ checkMenuOptions(expected);
+ }
+
+ @Test
+ @SmallTest
+ @UiThreadTest
+ public void testPageDownloadRestricted() {
+ FirstRunStatus.setFirstRunFlowComplete(true);
+ ContextMenuParams params =
+ new ContextMenuParams(
+ 0,
+ ContextMenuDataMediaType.NONE,
+ new GURL(PAGE_URL),
+ GURL.emptyGURL(),
+ "",
+ GURL.emptyGURL(),
+ GURL.emptyGURL(),
+ "",
+ null,
+ false,
+ 0,
+ 0,
+ MenuSourceType.TOUCH,
+ false,
+ false,
+ /* additionalNavigationParams= */ null);
+ DownloadUtils.setIsDownloadRestrictedByPolicyForTesting(true);
+
+ int[] expected = {R.id.contextmenu_save_page};
+ List<Integer> expected_disabled = Arrays.asList(R.id.contextmenu_save_page);
+
+ initializePopulatorOnDesktop(ChromeContextMenuPopulator.ContextMenuMode.NORMAL, params);
+ checkMenuOptions(expected_disabled, expected);
+
+ initializePopulatorOnDesktop(ChromeContextMenuPopulator.ContextMenuMode.CUSTOM_TAB, params);
+ checkMenuOptions(expected_disabled, expected);
+
+ initializePopulatorOnDesktop(ChromeContextMenuPopulator.ContextMenuMode.WEB_APP, params);
+ checkMenuOptions(expected_disabled, expected);
+
+ initializePopulatorOnDesktop(
+ ChromeContextMenuPopulator.ContextMenuMode.NETWORK_BOUND_TAB, params);
+ checkMenuOptions(expected_disabled, expected);
+ }
}
diff --git a/chrome/browser/ui/android/strings/android_chrome_strings.grd b/chrome/browser/ui/android/strings/android_chrome_strings.grd
index b303ffb..16d9c328 100644
--- a/chrome/browser/ui/android/strings/android_chrome_strings.grd
+++ b/chrome/browser/ui/android/strings/android_chrome_strings.grd
@@ -3095,6 +3095,9 @@
</message>
<!-- ContextMenu -->
+ <message name="IDS_CONTEXTMENU_SAVE_PAGE" desc="This string is shown in the context menu for saving the current page.">
+ Download
+ </message>
<message name="IDS_CONTEXTMENU_OPEN_IN_OTHER_WINDOW" desc="Context sensitive menu item to open the selected link in the other window. [CHAR_LIMIT=30]">
Open in other window
</message>
@@ -5538,6 +5541,9 @@
</message>
<!-- Custom Context Menu Informations -->
+ <message name="IDS_CONTEXTMENU_PAGE_TITLE" desc="The title of a context menu tab when the item pressed contains more than one type. This indicates that all the actions are related to the page.">
+ PAGE
+ </message>
<message name="IDS_CONTEXTMENU_IMAGE_TITLE" desc="The title of a context menu tab when the item pressed contains more than one type. This indicates that all the actions are related to the image.">
IMAGE
</message>
diff --git a/chrome/browser/ui/android/strings/android_chrome_strings_grd/IDS_CONTEXTMENU_SAVE_PAGE.png.sha1 b/chrome/browser/ui/android/strings/android_chrome_strings_grd/IDS_CONTEXTMENU_SAVE_PAGE.png.sha1
new file mode 100644
index 0000000..9ff3d4f
--- /dev/null
+++ b/chrome/browser/ui/android/strings/android_chrome_strings_grd/IDS_CONTEXTMENU_SAVE_PAGE.png.sha1
@@ -0,0 +1 @@
+b3564117e33bd2b1be95fde0eb074d1caf7597e5
\ No newline at end of file
diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
index 216f158..9a204cb8 100644
--- a/tools/metrics/histograms/enums.xml
+++ b/tools/metrics/histograms/enums.xml
@@ -2742,6 +2742,7 @@
<int value="38" label="Learn more about the shared highlighting feature"/>
<int value="39" label="Open in new tab in group"/>
<int value="40" label="Open in new window"/>
+ <int value="41" label="Save page"/>
</enum>
<!-- LINT.IfChange(ConversionCreateAggregatableReportStatus) -->
