Bug 388695 - getBookmarkIdsForURI should sort by relevance. r=dietrich, sayrer.

browser/components/places/tests/unit/test_placesTxn.js

@@ -149,21 +149,22 @@ function run_test() {
   // Create to a folder
   var txn2a = ptSvc.createFolder("Folder", root, bmStartIndex);
   var fldrId = bmsvc.getChildFolder(root, "Folder");
-  var txn2b = ptSvc.createItem(uri("http://www.example.com"), fldrId, bmStartIndex, "Testing1b");
-  ptSvc.commitTransaction(txn2); 
-  var b2 = (bmsvc.getBookmarkIdsForURI(uri("http://www.example.com"), {}))[0];
+  var txn2b = ptSvc.createItem(uri("http://www.example2.com"), fldrId, bmStartIndex, "Testing1b");
+  ptSvc.commitTransaction(txn2b);
+  var b2 = (bmsvc.getBookmarkIdsForURI(uri("http://www.example2.com"), {}))[0];
   do_check_eq(observer._itemAddedId, b2);
   do_check_eq(observer._itemAddedIndex, bmStartIndex);
-  do_check_true(bmsvc.isBookmarked(uri("http://www.example.com")));
+  do_check_true(bmsvc.isBookmarked(uri("http://www.example2.com")));
   txn2.undoTransaction();
   do_check_eq(observer._itemRemovedId, b2);
   do_check_eq(observer._itemRemovedIndex, bmStartIndex);
 
   // Testing moving an item
-  ptSvc.commitTransaction(ptSvc.createItem(uri("http://www.example.com"), root, -1, "Testing2"));
-  ptSvc.commitTransaction(ptSvc.createItem(uri("http://www.example.com"), root, -1, "Testing3"));   
-  ptSvc.commitTransaction(ptSvc.createItem(uri("http://www.example.com"), fldrId, -1, "Testing4"));
-  var bkmkIds = bmsvc.getBookmarkIdsForURI(uri("http://www.example.com"), {});
+  ptSvc.commitTransaction(ptSvc.createItem(uri("http://www.example3.com"), root, -1, "Testing2"));
+  ptSvc.commitTransaction(ptSvc.createItem(uri("http://www.example3.com"), root, -1, "Testing3"));   
+  ptSvc.commitTransaction(ptSvc.createItem(uri("http://www.example3.com"), fldrId, -1, "Testing4"));
+  var bkmkIds = bmsvc.getBookmarkIdsForURI(uri("http://www.example3.com"), {});
+  bkmkIds.sort();
   var bkmk1Id = bkmkIds[0];
   var bkmk2Id = bkmkIds[1];
   var bkmk3Id = bkmkIds[2];
@@ -266,7 +267,7 @@ function run_test() {
   txn9.undoTransaction();
   do_check_eq(observer._itemChangedId, bkmk1Id);
   do_check_eq(observer._itemChangedProperty, "uri");
-  do_check_eq(observer._itemChangedValue, "http://www.example.com/");
+  do_check_eq(observer._itemChangedValue, "http://www.example3.com/");
   
   // Test edit item description
   var txn10 = ptSvc.editItemDescription(bkmk1Id, "Description1");
@@ -345,6 +346,7 @@ function run_test() {
   ptSvc.commitTransaction(ptSvc.createItem(uri("http://www.sortingtest.com"), srtFldId, -1, "b"));   
   ptSvc.commitTransaction(ptSvc.createItem(uri("http://www.sortingtest.com"), srtFldId, -1, "a"));
   var b = bmsvc.getBookmarkIdsForURI(uri("http://www.sortingtest.com"), {});
+  b.sort();
   var b1 = b[0];
   var b2 = b[1];
   var b3 = b[2];

toolkit/components/places/public/nsITaggingService.idl

@@ -75,7 +75,8 @@ interface nsITaggingService : nsISupports
    * Retrieves all URLs tagged with the given tag.
    *
    * @param aTag
-   *        Array of uris tagged with aTag.
+   *        tag name
+   * @returns Array of uris tagged with aTag.
    */
   nsIVariant getURIsForTag(in AString aTag);
 
@@ -84,7 +85,7 @@ interface nsITaggingService : nsISupports
    *
    * @param aURI
    *        a URL.
-   * @returns array of tags.
+   * @returns array of tags (sorted by name).
    */
   nsIVariant getTagsForURI(in nsIURI aURI);
 };

toolkit/components/places/src/nsNavBookmarks.cpp

@@ -135,9 +135,10 @@ nsNavBookmarks::Init()
 
   // mDBFindURIBookmarks
   rv = dbConn->CreateStatement(NS_LITERAL_CSTRING(
-      "SELECT a.* "
+      "SELECT a.id "
       "FROM moz_bookmarks a, moz_places h "
-      "WHERE h.url = ?1 AND a.fk = h.id and a.type = ?2"),
+      "WHERE h.url = ?1 AND a.fk = h.id and a.type = ?2 "
+      "ORDER BY MAX(COALESCE(a.lastModified, 0), a.dateAdded) DESC"),
     getter_AddRefs(mDBFindURIBookmarks));
   NS_ENSURE_SUCCESS(rv, rv);
 

toolkit/components/places/src/nsTaggingService.js

@@ -262,11 +262,13 @@ TaggingService.prototype = {
           tags.push(this._tags[j].name);
       }
     }
+
+    // sort the tag list
+    tags.sort();
     return tags;
   }
 };
 
-
 var gModule = {
   registerSelf: function(componentManager, fileSpec, location, type) {
     componentManager = componentManager.QueryInterface(Ci.nsIComponentRegistrar);

toolkit/components/places/tests/bookmarks/test_388695.js

@@ -0,0 +1,84 @@
+/* -*- 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 *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is Bug 388695 unit test code.
+ *
+ * The Initial Developer of the Original Code is Mozilla Corporation.
+ * Portions created by the Initial Developer are Copyright (C) 2007
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *  Asaf Romano <mano@mozilla.com>
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+// Get bookmark service
+try {
+  var bmsvc = Cc["@mozilla.org/browser/nav-bookmarks-service;1"].
+              getService(Ci.nsINavBookmarksService);
+} catch(ex) {
+  do_throw("Could not get nav-bookmarks-service\n");
+}
+
+var gTestRoot;
+var gURI;
+var gItemId1;
+var gItemId2;
+
+// main
+function run_test() {
+  gURI = uri("http://foo.tld.com/");
+  gTestRoot = bmsvc.createFolder(bmsvc.placesRoot, "test folder",
+                                 bmsvc.DEFAULT_INDEX);
+
+  // test getBookmarkIdsForURI
+  // getBookmarkIdsForURI sorts by the most recently added/modified (descending)
+  //
+  // we cannot rely on dateAdded growing when doing so in a simple iteration,
+  // see PR_Now() documentation
+  do_test_pending();
+
+  gItemId1 = bmsvc.insertBookmark(gTestRoot, gURI, bmsvc.DEFAULT_INDEX, "");
+  do_timeout(100, "phase2();");
+}
+
+function phase2() {
+  gItemId2 = bmsvc.insertBookmark(gTestRoot, gURI, bmsvc.DEFAULT_INDEX, "");  
+  var b = bmsvc.getBookmarkIdsForURI(gURI, {});
+  do_check_eq(b[0], gItemId2);
+  do_check_eq(b[1], gItemId1);
+  do_timeout(100, "phase3();");
+}
+
+function phase3() {
+  // trigger last modified change
+  bmsvc.setItemTitle(gItemId1, "");
+  var b = bmsvc.getBookmarkIdsForURI(gURI, {});
+  do_check_eq(b[0], gItemId1);
+  do_check_eq(b[1], gItemId2);
+  do_test_finished();
+}

toolkit/components/places/tests/bookmarks/test_bookmarks.js

@@ -397,12 +397,6 @@ function run_test() {
   var u = bmsvc.getURIForKeyword("bar");
   do_check_eq("http://keywordtest.com/", u.spec);
 
-  // test getBookmarkIdsForURI
-  var newId8 = bmsvc.insertBookmark(testRoot, uri("http://foo8.com/"),
-                                    bmsvc.DEFAULT_INDEX, "");
-  var b = bmsvc.getBookmarkIdsForURI(uri("http://foo8.com/"), {});
-  do_check_eq(b[0], newId8);
-
   // test removeFolderChildren
   // 1) add/remove each child type (bookmark, separator, folder)
   var tmpFolder = bmsvc.createFolder(testRoot, "removeFolderChildren", bmsvc.DEFAULT_INDEX);
@@ -549,7 +543,7 @@ function run_test() {
                                      bmsvc.DEFAULT_INDEX, "");
   do_check_eq(observer._itemAddedId, newId13);
   do_check_eq(observer._itemAddedParent, testRoot);
-  do_check_eq(observer._itemAddedIndex, 13);
+  do_check_eq(observer._itemAddedIndex, 12);
 
   // set bookmark title
   bmsvc.setItemTitle(newId13, "ZZZXXXYYY");