Bug 1358554 - On Android devices that use static action bar prevent content from sliding down after a long press r=jchen
authorRandall Barker <rbarker@mozilla.com>
Fri, 21 Apr 2017 12:16:26 -0700
changeset 567889 127620ed468a54c89b0e865a423eaa713a04180b
parent 567888 3c9671705cdaca87c9bf94b2890108d62cacdc40
child 568123 85d3d629fa4a046a41a61de6012d9c880de1a594
push id55724
push userbmo:rbarker@mozilla.com
push dateTue, 25 Apr 2017 16:55:02 +0000
reviewersjchen
bugs1358554
milestone55.0a1
Bug 1358554 - On Android devices that use static action bar prevent content from sliding down after a long press r=jchen Only show real toolbar chrome for fixed action bar. Showing the static snapshot causes the content to shift. MozReview-Commit-ID: D4EN8qbe8Fu
mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
mobile/android/base/java/org/mozilla/gecko/DynamicToolbar.java
--- a/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
@@ -253,16 +253,22 @@ public class BrowserApp extends GeckoApp
     private int mCachedRecentTabsCount;
     private ActionModeCompat mActionMode;
     private TabHistoryController tabHistoryController;
 
     private static final int GECKO_TOOLS_MENU = -1;
     private static final int ADDON_MENU_OFFSET = 1000;
     public static final String TAB_HISTORY_FRAGMENT_TAG = "tabHistoryFragment";
 
+    // When the static action bar is shown, only the real toolbar chrome should be
+    // shown when the toolbar is visible. Causing the toolbar animator to also
+    // show the snapshot causes the content to shift under the users finger.
+    // See: Bug 1358554
+    private boolean mShowingToolbarChromeForActionBar;
+
     private static class MenuItemInfo {
         public int id;
         public String label;
         public boolean checkable;
         public boolean checked;
         public boolean enabled = true;
         public boolean visible = true;
         public int parent;
@@ -381,17 +387,20 @@ public class BrowserApp extends GeckoApp
                     tab.setShouldShowToolbarWithoutAnimationOnFirstSelection(false);
                 }
                 // fall through
             case LOCATION_CHANGE:
                 if (Tabs.getInstance().isSelectedTab(tab)) {
                     updateHomePagerForTab(tab);
                 }
 
-                mDynamicToolbar.persistTemporaryVisibility();
+                if (mShowingToolbarChromeForActionBar) {
+                    mDynamicToolbar.setVisible(true, VisibilityTransition.IMMEDIATE);
+                    mShowingToolbarChromeForActionBar = false;
+                }
                 break;
             case START:
                 if (Tabs.getInstance().isSelectedTab(tab)) {
                     invalidateOptionsMenu();
 
                     if (mDynamicToolbar.isEnabled()) {
                         mDynamicToolbar.setVisible(true, VisibilityTransition.ANIMATE);
                     }
@@ -4031,27 +4040,30 @@ public class BrowserApp extends GeckoApp
         return mReadingListHelper;
     }
 
     @Override
     protected ActionModePresenter getTextSelectPresenter() {
         return this;
     }
 
+
     /* Implementing ActionModeCompat.Presenter */
     @Override
     public void startActionMode(final ActionModeCompat.Callback callback) {
         // If actionMode is null, we're not currently showing one. Flip to the action mode view
         if (mActionMode == null) {
             mActionBarFlipper.showNext();
             DynamicToolbarAnimator toolbar = mLayerView.getDynamicToolbarAnimator();
 
-            // If the toolbar is dynamic and not currently showing, just slide it in
+            // If the toolbar is dynamic and not currently showing, just show the real toolbar
+            // and keep the animated snapshot hidden
             if (mDynamicToolbar.isEnabled() && toolbar.getCurrentToolbarHeight() == 0) {
-                mDynamicToolbar.setTemporarilyVisible(true, VisibilityTransition.ANIMATE);
+                toggleToolbarChrome(true);
+                mShowingToolbarChromeForActionBar = true;
             }
             mDynamicToolbar.setPinned(true, PinReason.ACTION_MODE);
 
         } else {
             // Otherwise, we're already showing an action mode. Just finish it and show the new one
             mActionMode.finish();
         }
 
@@ -4070,19 +4082,22 @@ public class BrowserApp extends GeckoApp
         }
 
         mActionMode.finish();
         mActionMode = null;
         mDynamicToolbar.setPinned(false, PinReason.ACTION_MODE);
 
         mActionBarFlipper.showPrevious();
 
-        // Only slide the urlbar out if it was hidden when the action mode started
-        // Don't animate hiding it so that there's no flash as we switch back to url mode
-        mDynamicToolbar.setTemporarilyVisible(false, VisibilityTransition.IMMEDIATE);
+        // Hide the real toolbar chrome if it was hidden before the action bar
+        // was shown.
+        if (mShowingToolbarChromeForActionBar) {
+            toggleToolbarChrome(false);
+            mShowingToolbarChromeForActionBar = false;
+        }
     }
 
     public static interface TabStripInterface {
         public void refresh();
         /** Called to let the tab strip know it is now, or is now no longer, being hidden by
          *  something being drawn over it.
          */
         void tabStripIsCovered(boolean covered);
--- a/mobile/android/base/java/org/mozilla/gecko/DynamicToolbar.java
+++ b/mobile/android/base/java/org/mozilla/gecko/DynamicToolbar.java
@@ -144,46 +144,16 @@ public class DynamicToolbar {
         final boolean isImmediate = transition == VisibilityTransition.IMMEDIATE;
         if (visible) {
             layerView.getDynamicToolbarAnimator().showToolbar(isImmediate);
         } else {
             layerView.getDynamicToolbarAnimator().hideToolbar(isImmediate);
         }
     }
 
-    public void setTemporarilyVisible(boolean visible, VisibilityTransition transition) {
-        ThreadUtils.assertOnUiThread();
-
-        if (layerView == null) {
-            return;
-        }
-
-        if (visible == temporarilyVisible) {
-            // nothing to do
-            return;
-        }
-
-        temporarilyVisible = visible;
-        final boolean isImmediate = transition == VisibilityTransition.IMMEDIATE;
-        if (visible) {
-            layerView.getDynamicToolbarAnimator().showToolbar(isImmediate);
-        } else {
-            layerView.getDynamicToolbarAnimator().hideToolbar(isImmediate);
-        }
-    }
-
-    public void persistTemporaryVisibility() {
-        ThreadUtils.assertOnUiThread();
-
-        if (temporarilyVisible) {
-            temporarilyVisible = false;
-            setVisible(true, VisibilityTransition.IMMEDIATE);
-        }
-    }
-
     public void setPinned(boolean pinned, PinReason reason) {
         ThreadUtils.assertOnUiThread();
         if (layerView == null) {
             return;
         }
 
         layerView.getDynamicToolbarAnimator().setPinned(pinned, reason);
     }