Bug 621843 - Fix View By Date and Site sorting. r=mak a=blocking-final
authorMehdi Mulani <mars.martian+bugmail@gmail.com>
Mon, 10 Jan 2011 10:11:31 -0800
changeset 60313 3ce1c7e22e856f439960f63f7bdf798fe229155a
parent 60312 eab687bc329f1072e9153eff446d3cfa14ca3c19
child 60314 08babae55fb42852772e6ac0b383d58b283e3a73
push idunknown
push userunknown
push dateunknown
reviewersmak, blocking-final
bugs621843
milestone2.0b10pre
Bug 621843 - Fix View By Date and Site sorting. r=mak a=blocking-final
toolkit/components/places/src/nsNavHistory.cpp
toolkit/components/places/tests/queries/head_queries.js
toolkit/components/places/tests/queries/test_sort-date-site-grouping.js
--- a/toolkit/components/places/src/nsNavHistory.cpp
+++ b/toolkit/components/places/src/nsNavHistory.cpp
@@ -3496,56 +3496,61 @@ PlacesSQLQueryBuilder::SelectAsSite()
   NS_ENSURE_STATE(history);
 
   history->GetStringFromName(NS_LITERAL_STRING("localhost").get(), localFiles);
   mAddParams.Put(NS_LITERAL_CSTRING("localhost"), localFiles);
 
   // If there are additional conditions the query has to join on visits too.
   nsCAutoString visitsJoin;
   nsCAutoString additionalConditions;
+  nsCAutoString timeConstraints;
   if (!mConditions.IsEmpty()) {
     visitsJoin.AssignLiteral("JOIN moz_historyvisits v ON v.place_id = h.id ");
     additionalConditions.AssignLiteral("{QUERY_OPTIONS_VISITS} "
                                        "{QUERY_OPTIONS_PLACES} "
                                        "{ADDITIONAL_CONDITIONS} ");
+    timeConstraints.AssignLiteral("||'&beginTime='||:begin_time||"
+                                    "'&endTime='||:end_time");
   }
 
   mQueryString = nsPrintfCString(2048,
-    "SELECT null, 'place:type=%ld&sort=%ld&domain=&domainIsHost=true', "
+    "SELECT null, 'place:type=%ld&sort=%ld&domain=&domainIsHost=true'%s, "
            ":localhost, :localhost, null, null, null, null, null, null, null "
     "WHERE EXISTS ( "
       "SELECT h.id FROM moz_places h "
       "%s "
       "WHERE h.hidden <> 1 "
         "AND h.visit_count > 0 "
         "AND h.url BETWEEN 'file://' AND 'file:/~' "
       "%s "
       "LIMIT 1 "
     ") "
     "UNION ALL "
     "SELECT null, "
-           "'place:type=%ld&sort=%ld&domain='||host||'&domainIsHost=true', "
+           "'place:type=%ld&sort=%ld&domain='||host||'&domainIsHost=true'%s, "
            "host, host, null, null, null, null, null, null, null "
     "FROM ( "
       "SELECT get_unreversed_host(h.rev_host) AS host "
       "FROM moz_places h "
       "%s "
       "WHERE h.hidden <> 1 "
         "AND h.rev_host <> '.' "
         "AND h.visit_count > 0 "
         "%s "
       "GROUP BY h.rev_host "
       "ORDER BY host ASC "
     ") ",
     nsINavHistoryQueryOptions::RESULTS_AS_URI,
     mSortingMode,
+    PromiseFlatCString(timeConstraints).get(),
     PromiseFlatCString(visitsJoin).get(),
     PromiseFlatCString(additionalConditions).get(),
     nsINavHistoryQueryOptions::RESULTS_AS_URI,
     mSortingMode,
+    PromiseFlatCString(timeConstraints).get(),
     PromiseFlatCString(visitsJoin).get(),
     PromiseFlatCString(additionalConditions).get()
   );
 
   return NS_OK;
 }
 
 nsresult
--- a/toolkit/components/places/tests/queries/head_queries.js
+++ b/toolkit/components/places/tests/queries/head_queries.js
@@ -56,16 +56,17 @@ let (commonFile = do_get_file("../head_c
 const DAY_MICROSEC = 86400000000;
 const today = Date.now() * 1000;
 const yesterday = today - DAY_MICROSEC;
 const lastweek = today - (DAY_MICROSEC * 7);
 const daybefore = today - (DAY_MICROSEC * 2);
 const tomorrow = today + DAY_MICROSEC;
 const old = today - (DAY_MICROSEC * 3);
 const futureday = today + (DAY_MICROSEC * 3);
+const olderthansixmonths = today - (DAY_MICROSEC * 31 * 7);
 
 
 /**
  * Generalized function to pull in an array of objects of data and push it into
  * the database. It does NOT do any checking to see that the input is
  * appropriate.
  */
 function populateDB(aArray) {
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/queries/test_sort-date-site-grouping.js
@@ -0,0 +1,207 @@
+/* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim:set ts=2 sw=2 sts=2 et: */
+/* ***** BEGIN LICENSE BLOCK *****
+  Any copyright is dedicated to the Public Domain.
+  http://creativecommons.org/publicdomain/zero/1.0/
+ * ***** END LICENSE BLOCK ***** */
+
+// This test ensures that the date and site type of |place:| query maintains
+// its quantifications correctly. Namely, it ensures that the date part of the
+// query is not lost when the domain queries are made.
+
+// We specifically craft these entries so that if a by Date and Site sorting is
+// applied, we find one domain in the today range, and two domains in the older
+// than six months range.
+// The correspondence between item in |testData| and date range is stored in
+// leveledTestData.
+let testData = [
+  {
+    isVisit: true,
+    uri: "file:///directory/1",
+    lastVisit: today,
+    isInQuery: true
+  },
+  {
+    isVisit: true,
+    uri: "http://example.com/1",
+    lastVisit: today,
+    isInQuery: true
+  },
+  {
+    isVisit: true,
+    uri: "http://example.com/2",
+    lastVisit: today,
+    isInQuery: true
+  },
+  {
+    isVisit: true,
+    uri: "file:///directory/2",
+    lastVisit: olderthansixmonths,
+    isInQuery: true
+  },
+  {
+    isVisit: true,
+    uri: "http://example.com/3",
+    lastVisit: olderthansixmonths,
+    isInQuery: true
+  },
+  {
+    isVisit: true,
+    uri: "http://example.com/4",
+    lastVisit: olderthansixmonths,
+    isInQuery: true
+  },
+  {
+    isVisit: true,
+    uri: "http://example.net/1",
+    lastVisit: olderthansixmonths + 1,
+    isInQuery: true
+  }
+];
+let domainsInRange = [2, 3];
+let leveledTestData = [// Today
+                       [[0],    // Today, local files
+                        [1,2]], // Today, example.com
+                       // Older than six months
+                       [[3],    // Older than six months, local files
+                        [4,5],  // Older than six months, example.com
+                        [6]     // Older than six months, example.net
+                        ]];
+
+// This test data is meant for live updating. The |levels| property indicates
+// date range index and then domain index.
+let testDataAddedLater = [
+  {
+    isVisit: true,
+    uri: "http://example.com/5",
+    lastVisit: olderthansixmonths,
+    isInQuery: true,
+    levels: [1,1]
+  },
+  {
+    isVisit: true,
+    uri: "http://example.com/6",
+    lastVisit: olderthansixmonths,
+    isInQuery: true,
+    levels: [1,1]
+  },
+  {
+    isVisit: true,
+    uri: "http://example.com/7",
+    lastVisit: today,
+    isInQuery: true,
+    levels: [0,1]
+  },
+  {
+    isVisit: true,
+    uri: "file:///directory/3",
+    lastVisit: today,
+    isInQuery: true,
+    levels: [0,0]
+  }
+];
+function run_test() {
+  populateDB(testData);
+
+  // On Linux, the (local files) folder is shown after sites unlike Mac/Windows.
+  // Thus, we avoid running this test on Linux but this should be re-enabled
+  // after bug 624024 is resolved.
+  let isLinux = ("@mozilla.org/gnome-gconf-service;1" in Components.classes);
+  if (isLinux)
+    return;
+
+  // In this test, there are three levels of results:
+  // 1st: Date queries. e.g., today, last week, or older than 6 months.
+  // 2nd: Domain queries restricted to a date. e.g. mozilla.com today.
+  // 3rd: Actual visits. e.g. mozilla.com/index.html today.
+  //
+  // We store all the third level result roots so that we can easily close all
+  // containers and test live updating into specific results.
+  let roots = [];
+
+  let query = PlacesUtils.history.getNewQuery();
+  let options = PlacesUtils.history.getNewQueryOptions();
+  options.resultType = Ci.nsINavHistoryQueryOptions.RESULTS_AS_DATE_SITE_QUERY;
+
+  let root = PlacesUtils.history.executeQuery(query, options).root;
+  root.containerOpen = true;
+
+  // This corresponds to the number of date ranges.
+  do_check_eq(root.childCount, leveledTestData.length);
+
+  // We pass off to |checkFirstLevel| to check the first level of results.
+  for (let index = 0; index < leveledTestData.length; index++) {
+    let node = root.getChild(index);
+    checkFirstLevel(index, node, roots);
+  }
+
+  // Test live updating.
+  testDataAddedLater.forEach(function(visit) {
+    populateDB([visit]);
+    let oldLength = testData.length;
+    let i = visit.levels[0];
+    let j = visit.levels[1];
+    testData.push(visit);
+    leveledTestData[i][j].push(oldLength);
+    compareArrayToResult(leveledTestData[i][j].
+                         map(function(x) testData[x]), roots[i][j]);
+  });
+
+  for (let i = 0; i < roots.length; i++) {
+    for (let j = 0; j < roots[i].length; j++)
+      roots[i][j].containerOpen = false;
+  }
+
+  root.containerOpen = false;
+}
+
+function checkFirstLevel(index, node, roots) {
+    node.containerOpen = true;
+
+    do_check_true(PlacesUtils.nodeIsDay(node));
+    PlacesUtils.asQuery(node);
+    let queries = node.getQueries();
+    let options = node.queryOptions;
+
+    do_check_eq(queries.length, 1);
+    let query = queries[0];
+
+    do_check_true(query.hasBeginTime && query.hasEndTime);
+
+    // Here we check the second level of results.
+    let root = PlacesUtils.history.executeQuery(query, options).root;
+    roots.push([]);
+    root.containerOpen = true;
+
+    do_check_eq(root.childCount, leveledTestData[index].length);
+    for (var secondIndex = 0; secondIndex < root.childCount; secondIndex++) {
+      let child = PlacesUtils.asQuery(root.getChild(secondIndex));
+      checkSecondLevel(index, secondIndex, child, roots);
+    }
+    root.containerOpen = false;
+    node.containerOpen = false;
+}
+
+function checkSecondLevel(index, secondIndex, child, roots) {
+    let queries = child.getQueries();
+    let options = child.queryOptions;
+
+    do_check_eq(queries.length, 1);
+    let query = queries[0];
+
+    do_check_true(query.hasDomain);
+    do_check_true(query.hasBeginTime && query.hasEndTime);
+
+    let root = PlacesUtils.history.executeQuery(query, options).root;
+    // We should now have that roots[index][secondIndex] is set to the second
+    // level's results root.
+    roots[index].push(root);
+
+    // We pass off to compareArrayToResult to check the third level of
+    // results.
+    root.containerOpen = true;
+    compareArrayToResult(leveledTestData[index][secondIndex].
+                         map(function(x) testData[x]), root);
+    // We close |root|'s container later so that we can test live
+    // updates into it.
+}