Bug 1339737 - Don't set the bookmarks panel scroll position again if the same loader has been reloaded. r?ahunt draft
authorJan Henning <jh+bugzilla@buttercookie.de>
Wed, 15 Feb 2017 21:48:29 +0100
changeset 484763 6f02ce3f3242b072a0e6f5ca8ad0d1732921a9db
parent 484736 e051804ab0552456b53ec111b3f1c8fa787d711c
child 545860 053001466532943d4b8e4088b4aefec76de9b0f8
push id45557
push usermozilla@buttercookie.de
push dateWed, 15 Feb 2017 21:00:15 +0000
reviewersahunt
bugs1339737
milestone54.0a1
Bug 1339737 - Don't set the bookmarks panel scroll position again if the same loader has been reloaded. r?ahunt Changes in the BrowserDB, e.g. because of sync or when opening a link (in a new tab) will trigger the BookmarksLoader's onContentChanged() method, which will trigger a new load reusing the current loader. This means that currently, the code for setting the scroll position in onLoadFinished() gets to run again in that case. We only want to set the scroll position when the user has navigated to a different folder. Folder navigation will always create a fresh loader, therefore we now keep track whether we've already seen a particular loader in onLoadFinished() and only set the scroll position if we're encountering this particular BookmarksLoader instance for the first time. MozReview-Commit-ID: Ln8yeUEoEfr
mobile/android/base/java/org/mozilla/gecko/home/BookmarksPanel.java
--- a/mobile/android/base/java/org/mozilla/gecko/home/BookmarksPanel.java
+++ b/mobile/android/base/java/org/mozilla/gecko/home/BookmarksPanel.java
@@ -67,16 +67,19 @@ public class BookmarksPanel extends Home
     private List<FolderInfo> mSavedParentStack;
 
     // Reference to the View to display when there are no results.
     private View mEmptyView;
 
     // Callback for cursor loaders.
     private CursorLoaderCallbacks mLoaderCallbacks;
 
+    // Keep track whether a fresh loader has been used or not.
+    private int mLastLoaderHash;
+
     @Override
     public void restoreData(@NonNull Bundle data) {
         final ArrayList<FolderInfo> stack = data.getParcelableArrayList("parentStack");
         if (stack == null) {
             return;
         }
 
         if (mListAdapter == null) {
@@ -313,18 +316,23 @@ public class BookmarksPanel extends Home
                 // is actually an unmodifiable wrapper around a LinkedList). We'll need to do a
                 // LinkedList conversion at the other end, when saving we need to use this awkward
                 // syntax to create an Array.
                 bundle.putParcelableArrayList("parentStack", new ArrayList<FolderInfo>(parentStack));
 
                 mPanelStateChangeListener.onStateChanged(bundle);
             }
 
-            if (mList != null) {
+            // BrowserDB updates (e.g. through sync, or when opening a new tab) will trigger
+            // a refresh which reuses the same loader - in that case we don't want to reset
+            // the scroll position again.
+            int currentLoaderHash = bl.hashCode();
+            if (mList != null && currentLoaderHash != mLastLoaderHash) {
                 mList.setSelection(bl.getTargetPosition());
+                mLastLoaderHash = currentLoaderHash;
             }
             updateUiFromCursor(c);
         }
 
         @Override
         public void onLoaderReset(Loader<Cursor> loader) {
             if (mList != null) {
                 mListAdapter.swapCursor(null);