Bug 1209967 - Toolbar: Always show the overflow menu. a=sylvestre
authorMichael Comella <michael.l.comella@gmail.com>
Tue, 17 Nov 2015 15:04:59 -0800
changeset 310459 892f22a8e3341161f051e2fb488cf07662e52109
parent 310458 270497f44addd845dbe2f0c28f30fe4526bcc830
child 310460 ce73e766b35f85ac985c669e7777d367d2c59f9d
push id5513
push userraliiev@mozilla.com
push dateMon, 25 Jan 2016 13:55:34 +0000
treeherdermozilla-beta@5ee97dd05b5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssylvestre
bugs1209967
milestone45.0a2
Bug 1209967 - Toolbar: Always show the overflow menu. a=sylvestre This required us to use the custom menu inflater and panels on GB. One side effect of this change is that the menu button no longer closes the menu on GB. I tried to fix this but BrowserApp.onKeyDown is not fired when the menu is open on GB and I'm not sure why that is. However, I don't think it's worth my time to fix since it's GB.
mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
mobile/android/base/java/org/mozilla/gecko/util/HardwareUtils.java
mobile/android/base/resources/menu/browser_app_menu.xml
--- a/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
@@ -344,21 +344,17 @@ public abstract class GeckoApp
 
         MenuInflater inflater = getMenuInflater();
         inflater.inflate(R.menu.gecko_app_menu, mMenu);
         return true;
     }
 
     @Override
     public MenuInflater getMenuInflater() {
-        if (Versions.feature11Plus) {
-            return new GeckoMenuInflater(this);
-        } else {
-            return super.getMenuInflater();
-        }
+        return new GeckoMenuInflater(this);
     }
 
     public MenuPanel getMenuPanel() {
         if (mMenuPanel == null) {
             onCreatePanelMenu(Window.FEATURE_OPTIONS_PANEL, null);
             invalidateOptionsMenu();
         }
         return mMenuPanel;
@@ -399,33 +395,33 @@ public abstract class GeckoApp
 
     @Override
     public void closeMenu() {
         closeOptionsMenu();
     }
 
     @Override
     public View onCreatePanelView(int featureId) {
-        if (Versions.feature11Plus && featureId == Window.FEATURE_OPTIONS_PANEL) {
+        if (featureId == Window.FEATURE_OPTIONS_PANEL) {
             if (mMenuPanel == null) {
                 mMenuPanel = new MenuPanel(this, null);
             } else {
                 // Prepare the panel every time before showing the menu.
                 onPreparePanel(featureId, mMenuPanel, mMenu);
             }
 
             return mMenuPanel;
         }
 
         return super.onCreatePanelView(featureId);
     }
 
     @Override
     public boolean onCreatePanelMenu(int featureId, Menu menu) {
-        if (Versions.feature11Plus && featureId == Window.FEATURE_OPTIONS_PANEL) {
+        if (featureId == Window.FEATURE_OPTIONS_PANEL) {
             if (mMenuPanel == null) {
                 mMenuPanel = (MenuPanel) onCreatePanelView(featureId);
             }
 
             GeckoMenu gMenu = new GeckoMenu(this, null);
             gMenu.setCallback(this);
             gMenu.setMenuPresenter(this);
             menu = gMenu;
@@ -434,31 +430,31 @@ public abstract class GeckoApp
             return onCreateOptionsMenu(menu);
         }
 
         return super.onCreatePanelMenu(featureId, menu);
     }
 
     @Override
     public boolean onPreparePanel(int featureId, View view, Menu menu) {
-        if (Versions.feature11Plus && featureId == Window.FEATURE_OPTIONS_PANEL) {
+        if (featureId == Window.FEATURE_OPTIONS_PANEL) {
             return onPrepareOptionsMenu(menu);
         }
 
         return super.onPreparePanel(featureId, view, menu);
     }
 
     @Override
     public boolean onMenuOpened(int featureId, Menu menu) {
         // exit full-screen mode whenever the menu is opened
         if (mLayerView != null && mLayerView.isFullScreen()) {
             GeckoAppShell.sendEventToGecko(GeckoEvent.createBroadcastEvent("FullScreen:Exit", null));
         }
 
-        if (Versions.feature11Plus && featureId == Window.FEATURE_OPTIONS_PANEL) {
+        if (featureId == Window.FEATURE_OPTIONS_PANEL) {
             if (mMenu == null) {
                 // getMenuPanel() will force the creation of the menu as well
                 MenuPanel panel = getMenuPanel();
                 onPreparePanel(featureId, panel, mMenu);
             }
 
             // Scroll custom menu to the top
             if (mMenuPanel != null)
@@ -513,20 +509,18 @@ public abstract class GeckoApp
             return true;
         }
 
         return super.onOptionsItemSelected(item);
     }
 
     @Override
     public void onOptionsMenuClosed(Menu menu) {
-        if (Versions.feature11Plus) {
-            mMenuPanel.removeAllViews();
-            mMenuPanel.addView((GeckoMenu) mMenu);
-        }
+        mMenuPanel.removeAllViews();
+        mMenuPanel.addView((GeckoMenu) mMenu);
     }
 
     @Override
     public boolean onKeyDown(int keyCode, KeyEvent event) {
         // Handle hardware menu key presses separately so that we can show a custom menu in some cases.
         if (keyCode == KeyEvent.KEYCODE_MENU) {
             openOptionsMenu();
             return true;
--- a/mobile/android/base/java/org/mozilla/gecko/util/HardwareUtils.java
+++ b/mobile/android/base/java/org/mozilla/gecko/util/HardwareUtils.java
@@ -85,17 +85,18 @@ public final class HardwareUtils {
         return sIsSmallTablet;
     }
 
     public static boolean isTelevision() {
         return sIsTelevision;
     }
 
     public static boolean hasMenuButton() {
-        return sHasMenuButton;
+        // TODO: Remove these calls.
+        return false;
     }
 
     public static int getMemSize() {
         return SysInfo.getMemSize();
     }
 
     public static boolean isLowMemoryPlatform() {
         final int memSize = getMemSize();
--- a/mobile/android/base/resources/menu/browser_app_menu.xml
+++ b/mobile/android/base/resources/menu/browser_app_menu.xml
@@ -4,16 +4,18 @@
    - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
 
 <menu xmlns:android="http://schemas.android.com/apk/res/android">
 
     <item android:id="@+id/reload"
           android:icon="@drawable/ic_menu_reload"
           android:title="@string/reload"/>
 
+    <!-- We keep the reference so calls to findView don't fail. Hide
+         to avoid taking up real estate on the users' screen. -->
     <item android:id="@+id/back"
           android:icon="@drawable/ic_menu_back"
           android:title="@string/back"
           android:visible="false"/>
 
     <item android:id="@+id/forward"
           android:icon="@drawable/ic_menu_forward"
           android:title="@string/forward"/>
@@ -31,69 +33,78 @@
           android:title="@string/new_private_tab"/>
 
     <item android:id="@+id/share"
           android:title="@string/share" />
 
     <item android:id="@+id/reading_list"
           android:title="@string/overlay_share_reading_list_btn_label" />
 
-    <item android:id="@+id/save_as_pdf"
-          android:title="@string/save_as_pdf" />
-
-    <item android:id="@+id/print"
-          android:visible="false"
-          android:title="@string/print" />
-
     <item android:id="@+id/find_in_page"
           android:title="@string/find_in_page" />
 
     <item android:id="@+id/desktop_mode"
           android:title="@string/desktop_mode"
           android:checkable="true" />
 
-    <item android:id="@+id/addons"
-          android:title="@string/addons"/>
+    <item android:id="@+id/page"
+          android:title="@string/page">
+
+        <menu>
+            <item android:id="@+id/subscribe"
+                  android:title="@string/contextmenu_subscribe"/>
+
+            <item android:id="@+id/save_as_pdf"
+                  android:title="@string/save_as_pdf"/>
+
+            <item android:id="@+id/print"
+                  android:title="@string/print"/>
+
+            <item android:id="@+id/add_search_engine"
+                  android:title="@string/contextmenu_add_search_engine"/>
+
+            <item android:id="@+id/add_to_launcher"
+                  android:title="@string/contextmenu_add_to_launcher"/>
+        </menu>
 
-    <item android:id="@+id/downloads"
-          android:title="@string/downloads"/>
+    </item>
+
+    <item android:id="@+id/tools"
+          android:title="@string/tools">
 
-    <item android:id="@+id/logins"
-          android:title="@string/logins"/>
+        <menu>
+            <item android:id="@+id/downloads"
+                  android:title="@string/downloads"/>
+            <item android:id="@+id/addons"
+                  android:title="@string/addons"/>
+            <item android:id="@+id/logins"
+                  android:title="@string/logins"/>
+            <item android:id="@+id/new_guest_session"
+                  android:visible="false"
+                  android:title="@string/new_guest_session"/>
+            <item android:id="@+id/exit_guest_session"
+                  android:visible="false"
+                  android:title="@string/exit_guest_session"/>
+        </menu>
+
+    </item>
 
     <item android:id="@+id/char_encoding"
           android:visible="false"
           android:title="@string/char_encoding"/>
 
     <item android:id="@+id/settings"
           android:title="@string/settings" />
 
     <item android:id="@+id/help"
           android:title="@string/help_menu" />
 
-    <item android:id="@+id/new_guest_session"
-          android:visible="false"
-          android:title="@string/new_guest_session"/>
-
-    <item android:id="@+id/exit_guest_session"
-          android:visible="false"
-          android:title="@string/exit_guest_session"/>
-
     <!-- Android will eliminate v11+ resource files from pre-11 builds.
          Those files are the only place in which certain IDs are defined.
          This causes compilation errors.
          To avoid nasty code workarounds, we define stub IDs here, but
          hide the menu entries. -->
 
     <item android:id="@+id/quickshare"
           android:visible="false"
           android:enabled="false" />
 
-    <item android:id="@+id/page"
-          android:visible="false"
-          android:enabled="false"
-          android:title="@string/page" />
-
-    <item android:id="@+id/tools"
-          android:visible="false"
-          android:enabled="false"
-          android:title="@string/tools" />
 </menu>