Bug 1869982 - With bookmarks toolbar set to show on new tab, it doesn…

…'t disappear after loading data URIs longer than 64KiB characters from the new tab page. r=mak,places-reviewers Differential Revision: https://phabricator.services.mozilla.com/D199980
dothq · Feb 5, 2024 · 2dc35f5 · 2dc35f5
1 parent 8c822cb
commit 2dc35f5
Show file tree

Hide file tree

Showing 3 changed files with 124 additions and 141 deletions.
diff --git a/toolkit/components/places/Bookmarks.sys.mjs b/toolkit/components/places/Bookmarks.sys.mjs
@@ -1494,7 +1494,7 @@ export var Bookmarks = Object.freeze({
    * @note Any unknown property in the info object is ignored.  Known properties
    *       may be overwritten.
    */
-  fetch(guidOrInfo, onResult = null, options = {}) {
+  async fetch(guidOrInfo, onResult = null, options = {}) {
     if (onResult && typeof onResult != "function") {
       throw new Error("onResult callback must be a valid function");
     }
@@ -1557,67 +1557,65 @@ export var Bookmarks = Object.freeze({
       behavior
     );
 
-    return (async () => {
-      let results;
-      if (fetchInfo.hasOwnProperty("url")) {
-        results = await fetchBookmarksByURL(fetchInfo, options);
-      } else if (fetchInfo.hasOwnProperty("guid")) {
-        results = await fetchBookmark(fetchInfo, options);
-      } else if (fetchInfo.hasOwnProperty("parentGuid")) {
-        if (fetchInfo.hasOwnProperty("index")) {
-          results = await fetchBookmarkByPosition(fetchInfo, options);
-        } else {
-          results = await fetchBookmarksByParentGUID(fetchInfo, options);
-        }
-      } else if (fetchInfo.hasOwnProperty("guidPrefix")) {
-        results = await fetchBookmarksByGUIDPrefix(fetchInfo, options);
-      } else if (fetchInfo.hasOwnProperty("tags")) {
-        results = await fetchBookmarksByTags(fetchInfo, options);
+    let results;
+    if (fetchInfo.hasOwnProperty("url")) {
+      results = await fetchBookmarksByURL(fetchInfo, options);
+    } else if (fetchInfo.hasOwnProperty("guid")) {
+      results = await fetchBookmark(fetchInfo, options);
+    } else if (fetchInfo.hasOwnProperty("parentGuid")) {
+      if (fetchInfo.hasOwnProperty("index")) {
+        results = await fetchBookmarkByPosition(fetchInfo, options);
+      } else {
+        results = await fetchBookmarksByParentGUID(fetchInfo, options);
       }
+    } else if (fetchInfo.hasOwnProperty("guidPrefix")) {
+      results = await fetchBookmarksByGUIDPrefix(fetchInfo, options);
+    } else if (fetchInfo.hasOwnProperty("tags")) {
+      results = await fetchBookmarksByTags(fetchInfo, options);
+    }
 
-      if (!results) {
-        return null;
-      }
+    if (!results) {
+      return null;
+    }
 
-      if (!Array.isArray(results)) {
-        results = [results];
+    if (!Array.isArray(results)) {
+      results = [results];
+    }
+    // Remove non-enumerable properties.
+    results = results.map(r => {
+      if (r.type == this.TYPE_FOLDER) {
+        r.childCount = r._childCount;
       }
-      // Remove non-enumerable properties.
-      results = results.map(r => {
-        if (r.type == this.TYPE_FOLDER) {
-          r.childCount = r._childCount;
-        }
-        if (options.includeItemIds) {
-          r.itemId = r._id;
-          r.parentId = r._parentId;
-        }
-        return Object.assign({}, r);
-      });
+      if (options.includeItemIds) {
+        r.itemId = r._id;
+        r.parentId = r._parentId;
+      }
+      return Object.assign({}, r);
+    });
 
-      if (options.includePath) {
-        for (let result of results) {
-          let folderPath = await retrieveFullBookmarkPath(result.parentGuid);
-          if (folderPath) {
-            result.path = folderPath;
-          }
+    if (options.includePath) {
+      for (let result of results) {
+        let folderPath = await retrieveFullBookmarkPath(result.parentGuid);
+        if (folderPath) {
+          result.path = folderPath;
         }
       }
+    }
 
-      // Ideally this should handle an incremental behavior and thus be invoked
-      // while we fetch.  Though, the likelihood of 2 or more bookmarks for the
-      // same match is very low, so it's not worth the added code complication.
-      if (onResult) {
-        for (let result of results) {
-          try {
-            onResult(result);
-          } catch (ex) {
-            console.error(ex);
-          }
+    // Ideally this should handle an incremental behavior and thus be invoked
+    // while we fetch.  Though, the likelihood of 2 or more bookmarks for the
+    // same match is very low, so it's not worth the added code complication.
+    if (onResult) {
+      for (let result of results) {
+        try {
+          onResult(result);
+        } catch (ex) {
+          console.error(ex);
         }
       }
+    }
 
-      return results[0];
-    })();
+    return results[0];
   },
 
   /**

diff --git a/toolkit/components/places/tests/bookmarks/test_bookmarks_fetch.js b/toolkit/components/places/tests/bookmarks/test_bookmarks_fetch.js
@@ -9,140 +9,125 @@ var gAccumulator = {
 };
 
 add_task(async function invalid_input_throws() {
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch(),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch(),
     /Input should be a valid object/
   );
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch(null),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch(null),
     /Input should be a valid object/
   );
-
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch({ guid: "123456789012", index: 0 }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({ guid: "123456789012", index: 0 }),
     /The following properties were expected: parentGuid/
   );
-
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch({}),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({}),
     /Unexpected number of conditions provided: 0/
   );
-  Assert.throws(
-    () =>
-      PlacesUtils.bookmarks.fetch({
-        type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
-      }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({
+      type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+    }),
     /Unexpected number of conditions provided: 0/
   );
-  Assert.throws(
-    () =>
-      PlacesUtils.bookmarks.fetch({
-        guid: "123456789012",
-        parentGuid: "012345678901",
-        index: 0,
-      }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({
+      guid: "123456789012",
+      parentGuid: "012345678901",
+      index: 0,
+    }),
     /Unexpected number of conditions provided: 2/
   );
-  Assert.throws(
-    () =>
-      PlacesUtils.bookmarks.fetch({
-        guid: "123456789012",
-        url: "http://example.com",
-      }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({
+      guid: "123456789012",
+      url: "http://example.com",
+    }),
     /Unexpected number of conditions provided: 2/
   );
-
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch("test"),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch("test"),
     /Invalid value for property 'guid'/
   );
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch(123),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch(123),
     /Invalid value for property 'guid'/
   );
-
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch({ guid: "test" }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({ guid: "test" }),
     /Invalid value for property 'guid'/
   );
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch({ guid: null }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({ guid: null }),
     /Invalid value for property 'guid'/
   );
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch({ guid: 123 }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({ guid: 123 }),
     /Invalid value for property 'guid'/
   );
-
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch({ guidPrefix: "" }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({ guidPrefix: "" }),
     /Invalid value for property 'guidPrefix'/
   );
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch({ guidPrefix: null }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({ guidPrefix: null }),
     /Invalid value for property 'guidPrefix'/
   );
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch({ guidPrefix: 123 }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({ guidPrefix: 123 }),
     /Invalid value for property 'guidPrefix'/
   );
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch({ guidPrefix: "123456789012" }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({ guidPrefix: "123456789012" }),
     /Invalid value for property 'guidPrefix'/
   );
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch({ guidPrefix: "@" }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({ guidPrefix: "@" }),
     /Invalid value for property 'guidPrefix'/
   );
-
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch({ parentGuid: "test", index: 0 }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({ parentGuid: "test", index: 0 }),
     /Invalid value for property 'parentGuid'/
   );
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch({ parentGuid: null, index: 0 }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({ parentGuid: null, index: 0 }),
     /Invalid value for property 'parentGuid'/
   );
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch({ parentGuid: 123, index: 0 }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({ parentGuid: 123, index: 0 }),
     /Invalid value for property 'parentGuid'/
   );
-
-  Assert.throws(
-    () =>
-      PlacesUtils.bookmarks.fetch({ parentGuid: "123456789012", index: "0" }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({ parentGuid: "123456789012", index: "0" }),
     /Invalid value for property 'index'/
   );
-  Assert.throws(
-    () =>
-      PlacesUtils.bookmarks.fetch({ parentGuid: "123456789012", index: null }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({ parentGuid: "123456789012", index: null }),
     /Invalid value for property 'index'/
   );
-  Assert.throws(
-    () =>
-      PlacesUtils.bookmarks.fetch({ parentGuid: "123456789012", index: -10 }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({ parentGuid: "123456789012", index: -10 }),
     /Invalid value for property 'index'/
   );
-
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch({ url: "http://te st/" }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({ url: "http://te st/" }),
     /Invalid value for property 'url'/
   );
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch({ url: null }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({ url: null }),
     /Invalid value for property 'url'/
   );
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch({ url: -10 }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({ url: -10 }),
     /Invalid value for property 'url'/
   );
-
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch("123456789012", "test"),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch("123456789012", "test"),
     /onResult callback must be a valid function/
   );
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch("123456789012", {}),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch("123456789012", {}),
     /onResult callback must be a valid function/
   );
 });

diff --git a/toolkit/components/places/tests/bookmarks/test_tags.js b/toolkit/components/places/tests/bookmarks/test_tags.js
@@ -34,24 +34,24 @@ add_task(async function test_fetchTags() {
 });
 
 add_task(async function test_fetch_by_tags() {
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch({ tags: "" }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({ tags: "" }),
     /Invalid value for property 'tags'/
   );
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch({ tags: [] }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({ tags: [] }),
     /Invalid value for property 'tags'/
   );
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch({ tags: null }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({ tags: null }),
     /Invalid value for property 'tags'/
   );
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch({ tags: [""] }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({ tags: [""] }),
     /Invalid value for property 'tags'/
   );
-  Assert.throws(
-    () => PlacesUtils.bookmarks.fetch({ tags: ["valid", null] }),
+  await Assert.rejects(
+    PlacesUtils.bookmarks.fetch({ tags: ["valid", null] }),
     /Invalid value for property 'tags'/
   );