1125116 - Remove keywords support from Bookmarks.jsm

Status: RESOLVED FIXED

(Toolkit :: Places, defect)

Description

[Marco Bonardo [:mak]]

The new API should be independent from keywords that will be moved to their own API.

Comment 1

[Marco Bonardo [:mak]]

Attachment: patch v1

This was pretty trivial

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8560492 [details] [diff] [review]
patch v1

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

::: toolkit/components/places/Bookmarks.jsm
@@ +444,5 @@
>     * Fetches information about a bookmark-item.
>     *
>     * REMARK: any successful call to this method resolves to a single
>     *         bookmark-item (or null), even when multiple bookmarks may exist
> +   *         (e.g. fetching by url ).  If you wish to retrieve all of the

Nit "url )." -> "url)."

::: toolkit/components/places/tests/bookmarks/test_bookmarks_eraseEverything.js
@@ -66,5 @@
>    Assert.equal(frecencyForUrl("http://example.com/"), frecencyForMozilla);
> -
> -  // Check there are no orphan keywords or annotations.
> -  let conn = yield PlacesUtils.promiseDBConnection();
> -  let rows = yield conn.execute(`SELECT * FROM moz_keywords`);

Should we keep checking for annotations here?

::: toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js
@@ -101,5 @@
> -  Assert.equal(bm2.keyword, "test");
> -
> -  // Check there are no orphan keywords or annotations.
> -  let conn = yield PlacesUtils.promiseDBConnection();
> -  let rows = yield conn.execute(`SELECT * FROM moz_keywords`);

Same here, keep the check for annotations?

Comment 3

[Marco Bonardo [:mak]]

(In reply to Tim Taubert [:ttaubert] from comment #2)
> Same here, keep the check for annotations?

whoops, yes.

Comment 4

[Marco Bonardo [:mak]]

https://hg.mozilla.org/integration/fx-team/rev/690f080dd893

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/690f080dd893