Bug 1263800 - Ensure bookmark star is enabled if tab data available r=grisha
authorAndrzej Hunt <ahunt@mozilla.com>
Thu, 14 Apr 2016 11:13:52 -0700
changeset 331294 1a22155b5544b1690270b7a3f9e93e495ed4cdd8
parent 331293 159d0cb3c2c360ec38ff965a50a67fc2b5ab736b
child 331295 11524fe96945c3c2e784db6ab3ddba10dbde3c29
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgrisha
bugs1263800
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1263800 - Ensure bookmark star is enabled if tab data available r=grisha If tab data is initially null then we disable the bookmark star. We need to explicitly reenable it when we load an actual page. All other menu items seem to be explicitly enabled as needed below, only bookmarks was omitted (since it's expected to be enabled ~all the time) - but we still could have disabled it previously. MozReview-Commit-ID: GpVJu4Rw2dN
mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
--- a/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
@@ -3214,16 +3214,18 @@ public class BrowserApp extends GeckoApp
         // In ICS+, it's easy to kill an app through the task switcher.
         final boolean visible = Versions.preICS ||
                                 HardwareUtils.isTelevision() ||
                                 !PrefUtils.getStringSet(GeckoSharedPrefs.forProfile(this),
                                                         ClearOnShutdownPref.PREF,
                                                         new HashSet<String>()).isEmpty();
         aMenu.findItem(R.id.quit).setVisible(visible);
 
+        // If tab data is unavailable we disable most of the context menu and related items and
+        // return early.
         if (tab == null || tab.getURL() == null) {
             bookmark.setEnabled(false);
             back.setEnabled(false);
             forward.setEnabled(false);
             share.setEnabled(false);
             quickShare.setEnabled(false);
             saveAsPDF.setEnabled(false);
             print.setEnabled(false);
@@ -3237,18 +3239,24 @@ public class BrowserApp extends GeckoApp
             }
             MenuUtils.safeSetEnabled(aMenu, R.id.subscribe, false);
             MenuUtils.safeSetEnabled(aMenu, R.id.add_search_engine, false);
             MenuUtils.safeSetEnabled(aMenu, R.id.add_to_launcher, false);
 
             return true;
         }
 
+        // If tab data IS available we need to manually enable items as necessary. They may have
+        // been disabled if returning early above, hence every item must be toggled, even if it's
+        // always expected to be enabled (e.g. the bookmark star is always enabled, except when
+        // we don't have tab data).
+
         final boolean inGuestMode = GeckoProfile.get(this).inGuestMode();
 
+        bookmark.setEnabled(true); // Might have been disabled above, ensure it's reenabled
         bookmark.setVisible(!inGuestMode);
         bookmark.setCheckable(true);
         bookmark.setChecked(tab.isBookmark());
         bookmark.setTitle(resolveBookmarkTitleID(tab.isBookmark()));
 
         if (Versions.feature11Plus) {
             // We don't use icons on GB builds so not resolving icons might conserve resources.
             bookmark.setIcon(resolveBookmarkIconID(tab.isBookmark()));