Index: chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java |
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 bf752e3ea1002fcd33d3a72c980ce8a23f4617c3..da96ebf7d58de440a670daccdbdcf7068b7c785e 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 |
@@ -10,7 +10,6 @@ import android.support.annotation.IntDef; |
import android.text.TextUtils; |
import android.view.ContextMenu; |
import android.view.MenuInflater; |
-import android.view.MenuItem; |
import android.webkit.MimeTypeMap; |
import org.chromium.base.metrics.RecordHistogram; |
@@ -24,7 +23,11 @@ import org.chromium.chrome.browser.util.UrlUtilities; |
import java.lang.annotation.Retention; |
import java.lang.annotation.RetentionPolicy; |
+import java.util.ArrayList; |
import java.util.Arrays; |
+import java.util.HashMap; |
+import java.util.List; |
+import java.util.Map; |
/** |
* A {@link ContextMenuPopulator} used for showing the default Chrome context menu. |
@@ -202,61 +205,92 @@ public class ChromeContextMenuPopulator implements ContextMenuPopulator { |
menu.setGroupVisible(R.id.contextmenu_group_image, params.isImage()); |
menu.setGroupVisible(R.id.contextmenu_group_video, params.isVideo()); |
+ boolean isGroupMessage = MailTo.isMailTo(params.getLinkUrl()) |
David Trainor- moved to gerrit
2017/02/22 05:36:33
Looks like we call these methods below too. Can w
JJ
2017/02/23 03:29:14
Sorry, missed this. Pushed it inside of the url.
|
+ || UrlUtilities.isTelScheme(params.getLinkUrl()); |
+ menu.setGroupVisible(R.id.contextmenu_group_message, isGroupMessage); |
+ |
+ Map<Integer, String> menuIdMap = new HashMap<>(menu.size()); |
Ted C
2017/02/22 06:10:54
Take a look at
https://developer.android.com/refe
JJ
2017/02/22 19:09:39
So I did look at this before I put it up thinking
|
+ for (int i = 0; i < menu.size(); i++) { |
+ if (menu.getItem(i).isVisible()) { |
David Trainor- moved to gerrit
2017/02/22 05:36:33
Can we pull out menu.getItem(i) to a local?
JJ
2017/02/22 19:09:39
Done.
|
+ menuIdMap.put(menu.getItem(i).getItemId(), (String) menu.getItem(i).getTitle()); |
+ } |
+ } |
+ menuIdMap = getListOfOptions(params, context, menuIdMap); |
David Trainor- moved to gerrit
2017/02/22 05:36:33
Do we have to return menuIdMap here? Might make i
Ted C
2017/02/22 06:10:54
looks like dtrainor@ left comments, but I'll put t
JJ
2017/02/22 19:09:39
No we don't need to return it at all! I'm hesitati
JJ
2017/02/22 19:09:39
I actually was thinking about this heavily when lo
|
+ |
+ // Iterate through the entire menu list, if if doesn't exist in the map, don't show it. |
+ for (int i = 0; i < menu.size(); i++) { |
+ boolean inMenuList = menuIdMap.containsKey(menu.getItem(i).getItemId()); |
David Trainor- moved to gerrit
2017/02/22 05:36:33
Pull out menu.getItem(i) again imo
JJ
2017/02/22 19:09:39
Done.
|
+ menu.getItem(i).setVisible(inMenuList); |
+ if (inMenuList) { |
+ menu.getItem(i).setTitle(menuIdMap.get(menu.getItem(i).getItemId())); |
+ } |
+ } |
+ } |
+ |
+ /** |
+ * Given a set of params and context, and a list of all the items, it'll return a list of items |
David Trainor- moved to gerrit
2017/02/22 05:36:33
remove it'll?
JJ
2017/02/22 19:09:38
Done.
|
+ * that should be displayed. This method never adds items, only removes or edits them from the |
+ * list. |
+ * @param params The parameters used for filtering out the the list of items. |
David Trainor- moved to gerrit
2017/02/22 05:36:33
Do we need @param context in here (even with no de
JJ
2017/02/22 19:09:38
Question for the future: Is it alright to have a p
|
+ * @param allItems A list, with all the possible options for a link. Items will be removed based |
David Trainor- moved to gerrit
2017/02/22 05:36:33
Can we describe what the format of the map is?
JJ
2017/02/22 19:09:38
Done.
|
+ * off the parameters. |
+ * @return Returns a map containing the id and string of all the items that are allowed to be |
+ * on the menu. |
+ */ |
+ private Map<Integer, String> getListOfOptions( |
+ ContextMenuParams params, Context context, Map<Integer, String> allItems) { |
if (params.isAnchor() && !mDelegate.isOpenInOtherWindowSupported()) { |
- menu.findItem(R.id.contextmenu_open_in_other_window).setVisible(false); |
+ allItems.remove(R.id.contextmenu_open_in_other_window); |
} |
if (mDelegate.isIncognito() || !mDelegate.isIncognitoSupported()) { |
- menu.findItem(R.id.contextmenu_open_in_incognito_tab).setVisible(false); |
+ allItems.remove(R.id.contextmenu_open_in_incognito_tab); |
} |
if (params.getLinkText().trim().isEmpty() || params.isImage()) { |
- menu.findItem(R.id.contextmenu_copy_link_text).setVisible(false); |
+ allItems.remove(R.id.contextmenu_copy_link_text); |
} |
if (params.isAnchor() && !UrlUtilities.isAcceptedScheme(params.getLinkUrl())) { |
- menu.findItem(R.id.contextmenu_open_in_other_window).setVisible(false); |
- menu.findItem(R.id.contextmenu_open_in_new_tab).setVisible(false); |
- menu.findItem(R.id.contextmenu_open_in_incognito_tab).setVisible(false); |
+ allItems.remove(R.id.contextmenu_open_in_other_window); |
+ allItems.remove(R.id.contextmenu_open_in_new_tab); |
+ allItems.remove(R.id.contextmenu_open_in_incognito_tab); |
} |
if (MailTo.isMailTo(params.getLinkUrl())) { |
- menu.findItem(R.id.contextmenu_copy_link_text).setVisible(false); |
- menu.findItem(R.id.contextmenu_copy_link_address).setVisible(false); |
- menu.setGroupVisible(R.id.contextmenu_group_message, true); |
+ allItems.remove(R.id.contextmenu_copy_link_text); |
+ allItems.remove(R.id.contextmenu_copy_link_address); |
if (!mDelegate.supportsSendEmailMessage()) { |
- menu.findItem(R.id.contextmenu_send_message).setVisible(false); |
+ allItems.remove(R.id.contextmenu_send_message); |
} |
if (TextUtils.isEmpty(MailTo.parse(params.getLinkUrl()).getTo()) |
|| !mDelegate.supportsAddToContacts()) { |
- menu.findItem(R.id.contextmenu_add_to_contacts).setVisible(false); |
+ allItems.remove(R.id.contextmenu_add_to_contacts); |
} |
- menu.findItem(R.id.contextmenu_call).setVisible(false); |
+ allItems.remove(R.id.contextmenu_call); |
} else if (UrlUtilities.isTelScheme(params.getLinkUrl())) { |
- menu.findItem(R.id.contextmenu_copy_link_text).setVisible(false); |
- menu.findItem(R.id.contextmenu_copy_link_address).setVisible(false); |
- menu.setGroupVisible(R.id.contextmenu_group_message, true); |
+ allItems.remove(R.id.contextmenu_copy_link_text); |
+ allItems.remove(R.id.contextmenu_copy_link_address); |
if (!mDelegate.supportsCall()) { |
- menu.findItem(R.id.contextmenu_call).setVisible(false); |
+ allItems.remove(R.id.contextmenu_call); |
} |
if (!mDelegate.supportsSendTextMessage()) { |
- menu.findItem(R.id.contextmenu_send_message).setVisible(false); |
+ allItems.remove(R.id.contextmenu_send_message); |
} |
if (!mDelegate.supportsAddToContacts()) { |
- menu.findItem(R.id.contextmenu_add_to_contacts).setVisible(false); |
+ allItems.remove(R.id.contextmenu_add_to_contacts); |
} |
- } else { |
- menu.setGroupVisible(R.id.contextmenu_group_message, false); |
} |
- menu.findItem(R.id.contextmenu_save_link_as).setVisible( |
- UrlUtilities.isDownloadableScheme(params.getLinkUrl())); |
+ if (!UrlUtilities.isDownloadableScheme(params.getLinkUrl())) { |
+ allItems.remove(R.id.contextmenu_save_link_as); |
+ } |
if (params.imageWasFetchedLoFi() |
|| !DataReductionProxySettings.getInstance().wasLoFiModeActiveOnMainFrame() |
|| !DataReductionProxySettings.getInstance().canUseDataReductionProxy( |
params.getPageUrl())) { |
- menu.findItem(R.id.contextmenu_load_images).setVisible(false); |
+ allItems.remove(R.id.contextmenu_load_images); |
} else { |
// Links can have images as backgrounds that aren't recognized here as images. CSS |
// properties can also prevent an image underlying a link from being clickable. |
@@ -267,26 +301,30 @@ public class ChromeContextMenuPopulator implements ContextMenuPopulator { |
} |
if (params.isVideo()) { |
- menu.findItem(R.id.contextmenu_save_video).setVisible( |
- params.canSaveMedia() && UrlUtilities.isDownloadableScheme(params.getSrcUrl())); |
+ boolean saveableAndDownloadable = |
+ params.canSaveMedia() && UrlUtilities.isDownloadableScheme(params.getSrcUrl()); |
David Trainor- moved to gerrit
2017/02/22 05:36:33
pull out isDownloadableScheme(params.getSrcUrl())
JJ
2017/02/23 03:29:14
Done.
|
+ if (!saveableAndDownloadable) { |
+ allItems.remove(R.id.contextmenu_save_video); |
+ } |
} else if (params.isImage() && params.imageWasFetchedLoFi()) { |
DataReductionProxyUma.previewsLoFiContextMenuAction( |
DataReductionProxyUma.ACTION_LOFI_LOAD_IMAGE_CONTEXT_MENU_SHOWN); |
// All image context menu items other than "Load image," "Open original image in |
// new tab," and "Copy image URL" should be disabled on Lo-Fi images. |
- menu.findItem(R.id.contextmenu_save_image).setVisible(false); |
- menu.findItem(R.id.contextmenu_open_image).setVisible(false); |
- menu.findItem(R.id.contextmenu_search_by_image).setVisible(false); |
- menu.findItem(R.id.contextmenu_share_image).setVisible(false); |
+ allItems.remove(R.id.contextmenu_save_image); |
+ allItems.remove(R.id.contextmenu_open_image); |
+ allItems.remove(R.id.contextmenu_search_by_image); |
+ allItems.remove(R.id.contextmenu_share_image); |
} else if (params.isImage() && !params.imageWasFetchedLoFi()) { |
- menu.findItem(R.id.contextmenu_load_original_image).setVisible(false); |
+ allItems.remove(R.id.contextmenu_load_original_image); |
- menu.findItem(R.id.contextmenu_save_image).setVisible( |
- UrlUtilities.isDownloadableScheme(params.getSrcUrl())); |
+ if (!UrlUtilities.isDownloadableScheme(params.getSrcUrl())) { |
+ allItems.remove(R.id.contextmenu_save_image); |
+ } |
// Avoid showing open image option for same image which is already opened. |
if (mDelegate.getPageUrl().equals(params.getSrcUrl())) { |
- menu.findItem(R.id.contextmenu_open_image).setVisible(false); |
+ allItems.remove(R.id.contextmenu_open_image); |
} |
final TemplateUrlService templateUrlServiceInstance = TemplateUrlService.getInstance(); |
final boolean isSearchByImageAvailable = |
@@ -296,43 +334,47 @@ public class ChromeContextMenuPopulator implements ContextMenuPopulator { |
&& templateUrlServiceInstance.getDefaultSearchEngineTemplateUrl() |
!= null; |
- menu.findItem(R.id.contextmenu_search_by_image).setVisible(isSearchByImageAvailable); |
if (isSearchByImageAvailable) { |
- menu.findItem(R.id.contextmenu_search_by_image).setTitle( |
+ allItems.put(R.id.contextmenu_search_by_image, |
David Trainor- moved to gerrit
2017/02/22 05:36:32
Does this break the rule? Seems like this adds th
JJ
2017/02/22 19:09:39
So the initial idea was to set up this method as a
|
context.getString(R.string.contextmenu_search_web_for_image, |
TemplateUrlService.getInstance() |
- .getDefaultSearchEngineTemplateUrl().getShortName())); |
+ .getDefaultSearchEngineTemplateUrl() |
+ .getShortName())); |
+ } else { |
+ allItems.remove(R.id.contextmenu_search_by_image); |
} |
} |
// Hide all items that could spawn additional tabs until FRE has been completed. |
if (!FirstRunStatus.getFirstRunFlowComplete()) { |
- menu.findItem(R.id.contextmenu_open_image_in_new_tab).setVisible(false); |
- menu.findItem(R.id.contextmenu_open_in_other_window).setVisible(false); |
- menu.findItem(R.id.contextmenu_open_in_new_tab).setVisible(false); |
- menu.findItem(R.id.contextmenu_open_in_incognito_tab).setVisible(false); |
- menu.findItem(R.id.contextmenu_search_by_image).setVisible(false); |
- menu.findItem(R.id.menu_id_open_in_chrome).setVisible(false); |
+ allItems.remove(R.id.contextmenu_open_image_in_new_tab); |
+ allItems.remove(R.id.contextmenu_open_in_other_window); |
+ allItems.remove(R.id.contextmenu_open_in_new_tab); |
+ allItems.remove(R.id.contextmenu_open_in_incognito_tab); |
+ allItems.remove(R.id.contextmenu_search_by_image); |
+ allItems.remove(R.id.menu_id_open_in_chrome); |
} |
if (mMode == FULLSCREEN_TAB_MODE) { |
- removeUnsupportedItems(menu, FULLSCREEN_TAB_MODE_WHITELIST); |
+ removeUnsupportedItems(allItems, FULLSCREEN_TAB_MODE_WHITELIST); |
} else if (mMode == CUSTOM_TAB_MODE) { |
- removeUnsupportedItems(menu, CUSTOM_TAB_MODE_WHITELIST); |
+ removeUnsupportedItems(allItems, CUSTOM_TAB_MODE_WHITELIST); |
} else { |
- removeUnsupportedItems(menu, NORMAL_MODE_WHITELIST); |
+ removeUnsupportedItems(allItems, NORMAL_MODE_WHITELIST); |
} |
+ |
+ return allItems; |
} |
- private void removeUnsupportedItems(ContextMenu menu, int[] whitelist) { |
+ private void removeUnsupportedItems(Map<Integer, String> itemList, int[] whitelist) { |
Arrays.sort(BASE_WHITELIST); |
Arrays.sort(whitelist); |
- for (int i = 0; i < menu.size(); i++) { |
- MenuItem item = menu.getItem(i); |
- if (Arrays.binarySearch(whitelist, item.getItemId()) < 0 |
- && Arrays.binarySearch(BASE_WHITELIST, item.getItemId()) < 0) { |
- menu.removeItem(item.getItemId()); |
- i--; |
+ List<Integer> keyList = new ArrayList<>(itemList.keySet()); |
+ for (int i = 0; i < keyList.size(); i++) { |
+ int itemId = keyList.get(i); |
+ if (Arrays.binarySearch(whitelist, itemId) < 0 |
+ && Arrays.binarySearch(BASE_WHITELIST, itemId) < 0) { |
+ itemList.remove(itemId); |
} |
} |
} |