Closed Bug 1388250 Opened 3 years ago Closed 3 years ago

Remove old sync isBookmarked and getBookmarkedURIFor APIs from nsINavBookmarksService

Categories

(Toolkit :: Places, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: standard8, Assigned: standard8)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file)

isBookmarked and getBookmarkedURIFor are only used in tests, and have been that way for a few releases.

As part of the work to move to the async APIs, and start removing the old ones (bug 1083465), we can drop these now.
Paolo: if you're not comfortable reviewing this, feel free to forward to Drew.
Comment on attachment 8894773 [details]
Bug 1388250 - Remove old sync isBookmarked and getBookmarkedURIFor APIs from nsINavBookmarksService.

https://reviewboard.mozilla.org/r/165942/#review171122

::: toolkit/components/places/tests/bookmarks/test_changeBookmarkURI.js:34
(Diff revision 1)
>    // Ensure that the URI corresponding to aBookmarkId equals aBookmarkedUri
>    do_check_true(bmsvc.getBookmarkURI(aBookmarkId).equals(aBookmarkedUri));
>  
>    // Ensure that aUnbookmarkedUri does not equal any URI that is bookmarked
> -  uri = bmsvc.getBookmarkedURIFor(aUnbookmarkedUri);
> -  do_check_eq(uri, null);
> +  bm = await PlacesUtils.bookmarks.fetch({url: aUnbookmarkedUri});
> +  do_check_eq(bm, null);

Sounds like you should choose between do_check_true/false(...) or do_check_eq/neq(..., null) throughout the tests. Since "fetch" returns null or the bookmark, probably the latter is better as it self-documents the function when reading the test.
Attachment #8894773 - Flags: review?(paolo.mozmail) → review+
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7f09351b3672
Remove old sync isBookmarked and getBookmarkedURIFor APIs from nsINavBookmarksService. r=Paolo
https://hg.mozilla.org/mozilla-central/rev/7f09351b3672
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.