1096856 - fetch({ url }) should exclude tags (like PlacesUtils.getBookmarksForURI)

Status: RESOLVED FIXED

(Toolkit :: Places, defect)

Description

[Asaf Romano (gone)]

Synchronous PlacesUtils.getBookmarksForURI excludes tags. Every caller that depends on this behavior (we have a couple dozens) cannot migrate to the new asynchronous fetch API, as it doesn't exclude tags from its search.

This also blocks porting PlacesTransactions.EditUrl to Bookmark.jsm.

Comment 1

[Asaf Romano (gone)]

Attachment: patch

Comment 2

[Marco Bonardo [:mak]]

Comment on attachment 8527629 [details] [diff] [review]
patch

Review of attachment 8527629 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/places/Bookmarks.jsm
@@ +967,5 @@
>  
>    return rows.length ? rowsToItemsArray(rows)[0] : null;
>  }
>  
> +function* fetchBookmarksByURL(info, excludeTags = false) {

per irc discussion, this should ALWAYS exclude tags

::: toolkit/components/places/tests/bookmarks/test_bookmarks_fetch.js
@@ +255,5 @@
>                                                   title: "a bookmark" });
>    checkBookmarkObject(bm1);
>  
> +  // Also ensure that fecth-by-url excludes the tags folder.
> +  PlacesUtils.tagging.tagURI(uri(bm1.url.href), ["Test Tag"]);

please untag before exiting the test, let's not risk to pollute next tests.

Comment 3

[Asaf Romano (gone)]

https://hg.mozilla.org/integration/fx-team/rev/409e4f16df0e

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/409e4f16df0e