Bug 1489060 - Urlbar autofill is broken when autofilling history is disabled and autofilling bookmarks is enabled r=mak a=pascalc
authorDrew Willcoxon <adw@mozilla.com>
Sat, 29 Sep 2018 08:53:42 +0000
changeset 492785 79aae3b724a5126554e76dbc93acf3c9805682fe
parent 492784 570f98364ad3221e2da465e866ec720c1f4b9d9d
child 492786 aa5533acaf04c5e66d920ffa144800d8c426d669
push id1815
push userffxbld-merge
push dateMon, 15 Oct 2018 10:40:45 +0000
treeherdermozilla-release@18d4c09e9378 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak, pascalc
bugs1489060
milestone63.0
Bug 1489060 - Urlbar autofill is broken when autofilling history is disabled and autofilling bookmarks is enabled r=mak a=pascalc In the origin query, the SELECT is aggregate due to both the TOTAL(frecency) column and the GROUP BY, but bookmarkedFragment is not aggregate. So it ends up looking only at whatever moz_place ends up first in its result set. If that moz_place's foreign_count is zero, then the whole bookmarkedFragment is zero, even if there are moz_places in the result set that are bookmarked. Change bookmarkedFragment to use TOTAL. Differential Revision: https://phabricator.services.mozilla.com/D6867
toolkit/components/places/UnifiedComplete.js
toolkit/components/places/tests/unifiedcomplete/test_autofill_origins.js
--- a/toolkit/components/places/UnifiedComplete.js
+++ b/toolkit/components/places/UnifiedComplete.js
@@ -324,24 +324,24 @@ function originQuery(conditions = "", bo
 const SQL_ORIGIN_QUERY = originQuery();
 
 const SQL_ORIGIN_PREFIX_QUERY = originQuery(
   `AND prefix BETWEEN :prefix AND :prefix || X'FFFF'`
 );
 
 const SQL_ORIGIN_BOOKMARKED_QUERY = originQuery(
   `AND bookmarked`,
-  `(SELECT foreign_count > 0 FROM moz_places
+  `(SELECT TOTAL(foreign_count) > 0 FROM moz_places
     WHERE moz_places.origin_id = moz_origins.id)`
 );
 
 const SQL_ORIGIN_PREFIX_BOOKMARKED_QUERY = originQuery(
   `AND bookmarked
    AND prefix BETWEEN :prefix AND :prefix || X'FFFF'`,
-  `(SELECT foreign_count > 0 FROM moz_places
+  `(SELECT TOTAL(foreign_count) > 0 FROM moz_places
     WHERE moz_places.origin_id = moz_origins.id)`
 );
 
 // Result row indexes for urlQuery()
 const QUERYINDEX_URL_URL = 1;
 const QUERYINDEX_URL_STRIPPED_URL = 2;
 const QUERYINDEX_URL_FRECENCY = 3;
 
--- a/toolkit/components/places/tests/unifiedcomplete/test_autofill_origins.js
+++ b/toolkit/components/places/tests/unifiedcomplete/test_autofill_origins.js
@@ -324,8 +324,140 @@ add_task(async function groupByHostNonDe
       },
     ],
   });
 
   Services.prefs.clearUserPref("browser.urlbar.autoFill.stddevMultiplier");
 
   await cleanup();
 });
+
+// This is similar to bookmarked() in autofill_tasks.js, but it adds
+// unbookmarked visits for multiple URLs with the same origin.
+add_task(async function bookmarkedMultiple() {
+  // Force only bookmarked pages to be suggested and therefore only bookmarked
+  // pages to be completed.
+  Services.prefs.setBoolPref("browser.urlbar.suggest.history", false);
+
+  let search = "ex";
+  let baseURL = "http://example.com/";
+  let bookmarkedURL = baseURL + "bookmarked";
+
+  // Add visits for three different URLs all sharing the same origin, and then
+  // bookmark the second one.  After that, the origin should be autofilled.  The
+  // reason for adding unbookmarked visits before and after adding the
+  // bookmarked visit is to make sure our aggregate SQL query for determining
+  // whether an origin is bookmarked is correct.
+
+  await PlacesTestUtils.addVisits([{
+    uri: baseURL + "other1",
+  }]);
+  await check_autocomplete({
+    search,
+    matches: [],
+  });
+
+  await PlacesTestUtils.addVisits([{
+    uri: bookmarkedURL,
+  }]);
+  await check_autocomplete({
+    search,
+    matches: [],
+  });
+
+  await PlacesTestUtils.addVisits([{
+    uri: baseURL + "other2",
+  }]);
+  await check_autocomplete({
+    search,
+    matches: [],
+  });
+
+  // Now bookmark the second URL.  It should be suggested and completed.
+  await addBookmark({
+    uri: bookmarkedURL,
+  });
+  await check_autocomplete({
+    search,
+    autofilled: "example.com/",
+    completed: baseURL,
+    matches: [
+      {
+        value: "example.com/",
+        comment: "example.com",
+        style: ["autofill", "heuristic"],
+      },
+      {
+        value: bookmarkedURL,
+        comment: "A bookmark",
+        style: ["bookmark"],
+      },
+    ],
+  });
+
+  await cleanup();
+});
+
+// This is similar to bookmarkedPrefix() in autofill_tasks.js, but it adds
+// unbookmarked visits for multiple URLs with the same origin.
+add_task(async function bookmarkedPrefixMultiple() {
+  // Force only bookmarked pages to be suggested and therefore only bookmarked
+  // pages to be completed.
+  Services.prefs.setBoolPref("browser.urlbar.suggest.history", false);
+
+  let search = "http://ex";
+  let baseURL = "http://example.com/";
+  let bookmarkedURL = baseURL + "bookmarked";
+
+  // Add visits for three different URLs all sharing the same origin, and then
+  // bookmark the second one.  After that, the origin should be autofilled.  The
+  // reason for adding unbookmarked visits before and after adding the
+  // bookmarked visit is to make sure our aggregate SQL query for determining
+  // whether an origin is bookmarked is correct.
+
+  await PlacesTestUtils.addVisits([{
+    uri: baseURL + "other1",
+  }]);
+  await check_autocomplete({
+    search,
+    matches: [],
+  });
+
+  await PlacesTestUtils.addVisits([{
+    uri: bookmarkedURL,
+  }]);
+  await check_autocomplete({
+    search,
+    matches: [],
+  });
+
+  await PlacesTestUtils.addVisits([{
+    uri: baseURL + "other2",
+  }]);
+  await check_autocomplete({
+    search,
+    matches: [],
+  });
+
+  // Now bookmark the second URL.  It should be suggested and completed.
+  await addBookmark({
+    uri: bookmarkedURL,
+  });
+  await check_autocomplete({
+    search,
+    autofilled: "http://example.com/",
+    completed: baseURL,
+    matches: [
+      {
+        value: "http://example.com/",
+        comment: "example.com",
+        style: ["autofill", "heuristic"],
+      },
+      {
+        value: bookmarkedURL,
+        comment: "A bookmark",
+        style: ["bookmark"],
+      },
+    ],
+  });
+
+  await cleanup();
+});