From 3610d06239e72bb51e4150889864d2b2c18347d5 Mon Sep 17 00:00:00 2001
From: Oren Blasberg <orenb@google.com>
Date: Thu, 14 Apr 2016 11:14:09 -0700
Subject: [PATCH] Don't show icons in overlay popup menus.

This was a "regression" in CascadingMenuPopup specifically.
We now check for the overflow case and don't show icons.
This preserves the behavior from Marshmallow.

Bug: 28026351
Change-Id: Ifbc9a20b0dadd19ef7b727023b1b0cfa45ebf993
---
 .../view/menu/CascadingMenuPopup.java         | 21 ++++++++++++-------
 .../android/internal/view/menu/MenuPopup.java |  5 +++++
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/core/java/com/android/internal/view/menu/CascadingMenuPopup.java b/core/java/com/android/internal/view/menu/CascadingMenuPopup.java
index ddca51fab97e5..5cb307fec6974 100644
--- a/core/java/com/android/internal/view/menu/CascadingMenuPopup.java
+++ b/core/java/com/android/internal/view/menu/CascadingMenuPopup.java
@@ -347,14 +347,21 @@ final class CascadingMenuPopup extends MenuPopup implements MenuPresenter, OnKey
         final LayoutInflater inflater = LayoutInflater.from(mContext);
         final MenuAdapter adapter = new MenuAdapter(menu, inflater, mOverflowOnly);
 
-        // Apply "force show icon" setting; if the menu being shown is the top level menu, apply the
-        // setting set on this CascadingMenuPopup by its creating code. If it's a submenu, or if no
-        // "force" setting was explicitly set, determine the setting by examining the items.
+        // Apply "force show icon" setting. There are 4 cases:
+        // (1) This is the top level menu. Only add spacing for icons if forced.
+        // (2) This is a submenu. Add spacing if any of the visible menu items has an icon.
+        // (3) This is a top level menu that is not an overflow menu. Add spacing if any of the
+        //     visible menu items has an icon.
+        // (4) This is an overflow menu or a top level menu that doesn't have "force" set.
+        //     Don't allow spacing.
         if (!isShowing() && mForceShowIcon) {
-            adapter.setForceShowIcon(mForceShowIcon);
-        } else {
-            adapter.setForceShowIcon(MenuPopup.shouldPreserveIconSpacing(menu));
+          // Case 1
+          adapter.setForceShowIcon(true);
+        } else if (isShowing() || !isShowing() && !mForceShowIcon && !mOverflowOnly) {
+          // Case 2 or 3
+          adapter.setForceShowIcon(MenuPopup.shouldPreserveIconSpacing(menu));
         }
+        // Case 4: Else, don't allow spacing for icons.
 
         final int menuWidth = measureIndividualMenuWidth(adapter, null, mContext, mMenuMaxWidth);
         final MenuPopupWindow popupWindow = createPopupWindow();
@@ -734,4 +741,4 @@ final class CascadingMenuPopup extends MenuPopup implements MenuPresenter, OnKey
             return window.getListView();
         }
     }
-}
\ No newline at end of file
+}
diff --git a/core/java/com/android/internal/view/menu/MenuPopup.java b/core/java/com/android/internal/view/menu/MenuPopup.java
index 16e4156b6336e..10bd66f178706 100644
--- a/core/java/com/android/internal/view/menu/MenuPopup.java
+++ b/core/java/com/android/internal/view/menu/MenuPopup.java
@@ -187,6 +187,11 @@ public abstract class MenuPopup implements ShowableListMenu, MenuPresenter,
     /**
      * Returns whether icon spacing needs to be preserved for the given menu, based on whether any
      * of its items contains an icon.
+     *
+     * NOTE: This should only be used for non-overflow-only menus, because this method does not
+     * take into account whether the menu items are being shown as part of the popup or or being
+     * shown as actions in the action bar.
+     *
      * @param menu
      * @return Whether to preserve icon spacing.
      */