Closed Bug 1096856 Opened 10 years ago Closed 9 years ago

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

Categories

(Toolkit :: Places, defect)

defect
Not set
normal
Points:
3

Tracking

()

RESOLVED FIXED
mozilla36
Iteration:
37.1

People

(Reporter: asaf, Assigned: asaf)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
Points: --- → 2
Flags: qe-verify-
Flags: in-testsuite?
Flags: firefox-backlog+
Points: 2 → 3
Attached patch patchSplinter Review
Assignee: nobody → mano
Status: NEW → ASSIGNED
Attachment #8527629 - Flags: review?(mak77)
Iteration: --- → 36.3
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.
Attachment #8527629 - Flags: review?(mak77) → review+
Iteration: 36.3 → 37.1
https://hg.mozilla.org/mozilla-central/rev/409e4f16df0e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.