Bug 1767434 - Make updateDetailsPane always retrieve treeIndex r=mak

The problem with my previous solution for bug 1471546 was that bookmarkIndex is
not updated when a user sorts the list of bookmarks. So my solution is to always
use the treeIndex since its more reliable.

I've also added two tests: the first checks if the row the visible after clicking
a bookmark that's close to the bottom of the viewable area, and the second does the
same after re-sorting the list.

Differential Revision: https://phabricator.services.mozilla.com/D145509

browser/components/places/content/places.js

@@ -448,17 +448,9 @@ var PlacesOrganizer = {
         .promiseDocumentFlushed(() => {})
         .then(() => {
           if (view.selectedNode && ContentArea.currentView.view) {
-            // When looking at a list of bookmarks/folders,
-            // bookmarkIndex can be considered the row number.
-            // In other contexts (Tags/History/Downloads), calculate the
-            // row using the node even if some of the items in the
-            // list happen to be bookmarks.
-            let row =
-              view.selectedNode.bookmarkIndex !== -1
-                ? view.selectedNode.bookmarkIndex
-                : ContentArea.currentView.view.treeIndexForNode(
-                    view.selectedNode
-                  );
+            let row = ContentArea.currentView.view.treeIndexForNode(
+              view.selectedNode
+            );
             ContentTree.view.ensureRowIsVisible(row);
           }
         });

browser/components/places/tests/browser/browser.ini

@@ -120,6 +120,7 @@ skip-if =
 [browser_library_open_all.js]
 [browser_library_open_bookmark.js]
 [browser_library_panel_leak.js]
+[browser_library_row_is_visible.js]
 [browser_library_search.js]
 [browser_library_tree_leak.js]
 [browser_library_views_liveupdate.js]

browser/components/places/tests/browser/browser_library_row_is_visible.js

@@ -0,0 +1,118 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+/**
+ * Checks whether or not a row is visible when a user clicks on
+ * row that's close to the bottom of the Library window, as the
+ * DetailsPane can grow after the user has clicked on a bookmark.
+ *
+ */
+"use strict";
+
+let library;
+let PlacesOrganizer;
+
+function assertSelectedRowIsVisible(selectedRow, msg) {
+  let firstRow = library.ContentTree.view.getFirstVisibleRow();
+  let lastRow = library.ContentTree.view.getLastVisibleRow();
+  Assert.ok(firstRow <= selectedRow && lastRow >= selectedRow, msg);
+}
+
+/**
+ * Add a bunch of bookmarks so that the Library view needs to be
+ * scrolled in order to see all the bookmarks.
+ */
+add_setup(async function() {
+  await PlacesUtils.bookmarks.eraseEverything();
+
+  library = await promiseLibrary("UnfiledBookmarks");
+
+  let baseUrl = "https://www.example.com/";
+
+  // Enough bookmarks that should go beyond the initial screen
+  let nBookmarks = library.ContentTree.view.getLastVisibleRow() + 5;
+
+  let bookmarks = new Array(nBookmarks);
+
+  // Hack to make it so that when the list of bookmarks is
+  // first loaded, a bookmark folder is first selected so that
+  // the details pane is small
+  bookmarks[0] = {
+    title: "Test Folder",
+    type: PlacesUtils.bookmarks.TYPE_FOLDER,
+  };
+
+  for (let index = 1; index < nBookmarks; ++index) {
+    bookmarks[index] = {
+      title: `Example Bookmark ${index + 10}`,
+      url: baseUrl + index + 10,
+      type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+    };
+  }
+
+  await PlacesUtils.bookmarks.insertTree({
+    guid: PlacesUtils.bookmarks.unfiledGuid,
+    children: bookmarks,
+  });
+
+  await promiseLibraryClosed(library);
+
+  registerCleanupFunction(async () => {
+    await PlacesUtils.bookmarks.eraseEverything();
+    await promiseLibraryClosed(library);
+  });
+});
+
+/**
+ * Click on a bookmark that should be hidden when the details
+ * panel is expanded, and so the library panel should scroll.
+ */
+add_task(async function test_click_bookmark() {
+  library = await promiseLibrary("UnfiledBookmarks");
+
+  let selectedRow = library.ContentTree.view.getLastVisibleRow();
+
+  let node = library.ContentTree.view.view.nodeForTreeIndex(selectedRow);
+  library.ContentTree.view.selectNode(node);
+  synthesizeClickOnSelectedTreeCell(library.ContentTree.view);
+
+  // TODO Bug 1769312: Have to wait till the row is scrolled to
+  // which unfortunately is not easy to know at the current moment.
+  // Should be replaced with an event.
+  /* eslint-disable mozilla/no-arbitrary-setTimeout */
+  await new Promise(resolve => {
+    setTimeout(resolve, 500);
+  });
+
+  assertSelectedRowIsVisible(selectedRow, "Selected row is visible");
+
+  await promiseLibraryClosed(library);
+});
+
+/**
+ * Sort a bookmarks list by one of the columns and check if
+ * clicking on a bookmark will show the bookmark.
+ */
+add_task(async function test_click_bookmark_on_sort() {
+  library = await promiseLibrary("UnfiledBookmarks");
+
+  let selectedRow = library.ContentTree.view.getLastVisibleRow();
+
+  // Re-sort by name
+  library.ViewMenu.setSortColumn(0, "descending");
+
+  let node = library.ContentTree.view.view.nodeForTreeIndex(selectedRow);
+  library.ContentTree.view.selectNode(node);
+  synthesizeClickOnSelectedTreeCell(library.ContentTree.view);
+
+  // TODO Bug 1769312: Have to wait till the row is scrolled to
+  // which unfortunately is not easy to know at the current moment.
+  // Should be replaced with an event.
+  /* eslint-disable mozilla/no-arbitrary-setTimeout */
+  await new Promise(resolve => {
+    setTimeout(resolve, 500);
+  });
+
+  assertSelectedRowIsVisible(selectedRow, "Selected row is visible after sort");
+});