Bug 1468104 - Existence of tag on a bookmark prevents deletion of associated keyword. r=mak
authorRob Hoelz <rob@hoelz.ro>
Tue, 19 Jun 2018 16:40:08 +0200
changeset 422965 e717472a6ac6ad0138932027312df3ab9e50afef
parent 422964 645e1b5b6470312fccafb126a1678b939f0d2b7f
child 422966 3f6f6020896579a4a2f8201638741ebd10d263b0
push id65313
push usermak77@bonardo.net
push dateTue, 19 Jun 2018 15:18:41 +0000
treeherderautoland@e717472a6ac6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1468104
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1468104 - Existence of tag on a bookmark prevents deletion of associated keyword. r=mak MozReview-Commit-ID: B0w1dZhVBWw
toolkit/components/places/PlacesUtils.jsm
toolkit/components/places/tests/unit/test_keywords.js
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -2488,17 +2488,22 @@ PlacesUtils.keywords = {
         }
 
         let placeInfosToRemove = [];
         let rows = await db.execute(`
           SELECT h.id, h.url
           FROM moz_places h
           JOIN moz_keywords k ON k.place_id = h.id
           GROUP BY h.id
-          HAVING h.foreign_count = COUNT(*)`);
+          HAVING h.foreign_count = count(*) +
+            (SELECT count(*)
+             FROM moz_bookmarks b
+             JOIN moz_bookmarks p ON b.parent = p.id
+             WHERE p.parent = :tags_root AND b.fk = h.id)
+          `, { tags_root: PlacesUtils.tagsFolderId });
         for (let row of rows) {
           placeInfosToRemove.push({
             placeId: row.getResultByName("id"),
             href: row.getResultByName("url"),
           });
         }
         if (!placeInfosToRemove.length) {
           return;
--- a/toolkit/components/places/tests/unit/test_keywords.js
+++ b/toolkit/components/places/tests/unit/test_keywords.js
@@ -486,8 +486,38 @@ add_task(async function test_bookmarkURL
                                        url: "http://example2.com/"});
   await promiseKeyword("keyword", "http://example2.com/");
 
   await check_keyword(false, "http://example1.com/", "keyword");
   await check_keyword(true, "http://example2.com/", "keyword");
   Assert.equal((await foreign_count("http://example1.com/")), fc1); // -1 bookmark -1 keyword
   Assert.equal((await foreign_count("http://example2.com/")), fc2 + 2); // +1 bookmark +1 keyword
 });
+
+add_task(async function test_tagDoesntPreventKeywordRemoval() {
+  await check_keyword(false, "http://example.com/", "example");
+  let fc = await foreign_count("http://example.com/");
+
+  let httpBookmark = await PlacesUtils.bookmarks.insert({
+      url: "http://example.com/",
+      parentGuid: PlacesUtils.bookmarks.unfiledGuid
+  });
+  Assert.equal((await foreign_count("http://example.com/")), fc + 1); // +1 bookmark
+
+  PlacesUtils.tagging.tagURI(uri("http://example.com/"), ["example_tag"]);
+  Assert.equal((await foreign_count("http://example.com/")), fc + 2); // +1 bookmark +1 tag
+
+  await PlacesUtils.keywords.insert({ keyword: "example", url: "http://example.com/" });
+  Assert.equal((await foreign_count("http://example.com/")), fc + 3); // +1 bookmark +1 tag +1 keyword
+
+  await check_keyword(true, "http://example.com/", "example");
+
+  await PlacesUtils.bookmarks.remove(httpBookmark);
+
+  await TestUtils.waitForCondition(
+      async () => !(await PlacesUtils.bookmarks.fetch({ url: "http://example.com/"})),
+      "Wait for bookmark to be removed");
+
+  await check_keyword(false, "http://example.com/", "example");
+  Assert.equal((await foreign_count("http://example.com/")), fc); // bookmark, keyword, and tag should all have been removed
+
+  await check_no_orphans();
+});