Bug 431153 - Middle clicking on a tag in the Library left pane does not open in tabs, r=dietrich
authorMarco Bonardo <mbonardo@mozilla.com>
Thu, 05 Mar 2009 13:08:18 +0100
changeset 25759 82d502128473889bcd0779e25c9a801bbeffbb33
parent 25758 85cad64285f9a8bef04f33f341c3771f6b39837c
child 25760 480d7c7f5c38d68ad3ce27eb22e1a3efb9f1e4fd
push id5729
push usermak77@bonardo.net
push dateThu, 05 Mar 2009 12:09:27 +0000
treeherdermozilla-central@255494d09521 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdietrich
bugs431153
milestone1.9.2a1pre
Bug 431153 - Middle clicking on a tag in the Library left pane does not open in tabs, r=dietrich
toolkit/components/places/src/utils.js
toolkit/components/places/tests/unit/test_utils_getURLsForContainerNode.js
--- a/toolkit/components/places/src/utils.js
+++ b/toolkit/components/places/src/utils.js
@@ -914,94 +914,123 @@ var PlacesUtils = {
     var livemarks = annosvc.getItemsWithAnnotation(LMANNO_FEEDURI, {});
     for (var i = 0; i < livemarks.length; i++) {
       if (annosvc.getItemAnnotation(livemarks[i], LMANNO_FEEDURI) == feedSpec)
         return livemarks[i];
     }
     return -1;
   },
 
-  // Returns true if a container has uris in its first level
-  // Has better performances than checking getURLsForContainerNode(node).length
+  /**
+   * Returns a nsNavHistoryContainerResultNode with forced excludeItems and
+   * expandQueries.
+   * @param   aNode
+   *          The node to convert
+   * @param   [optional] excludeItems
+   *          True to hide all items (individual bookmarks). This is used on
+   *          the left places pane so you just get a folder hierarchy.
+   * @param   [optional] expandQueries
+   *          True to make query items expand as new containers. For managing,
+   *          you want this to be false, for menus and such, you want this to
+   *          be true.
+   * @returns A nsINavHistoryContainerResultNode containing the unfiltered
+   *          contents of the container.
+   * @note    The returned container node could be open or closed, we don't
+   *          guarantee its status.
+   */
+  getContainerNodeWithOptions:
+  function PU_getContainerNodeWithOptions(aNode, aExcludeItems, aExpandQueries) {
+    if (!this.nodeIsContainer(aNode))
+      throw Cr.NS_ERROR_INVALID_ARG;
+
+    // excludeItems is inherited by child containers in an excludeItems view.
+    var excludeItems = asQuery(aNode).queryOptions.excludeItems ||
+                       asQuery(aNode.parentResult.root).queryOptions.excludeItems;
+    // expandQueries is inherited by child containers in an expandQueries view.
+    var expandQueries = asQuery(aNode).queryOptions.expandQueries &&
+                        asQuery(aNode.parentResult.root).queryOptions.expandQueries;
+
+    // If our options are exactly what we expect, directly return the node.
+    if (excludeItems == aExcludeItems && expandQueries == aExpandQueries)
+      return aNode;
+
+    // Otherwise, get contents manually.
+    var queries = {}, options = {};
+    this.history.queryStringToQueries(aNode.uri, queries, {}, options);
+    options.value.excludeItems = aExcludeItems;
+    options.value.expandQueries = aExpandQueries;
+    return this.history.executeQueries(queries.value,
+                                       queries.value.length,
+                                       options.value).root;
+  },
+
+  /**
+   * Returns true if a container has uri nodes in its first level.
+   * Has better performance than (getURLsForContainerNode(node).length > 0).
+   * @param aNode
+   *        The container node to search through.
+   * @returns true if the node contains uri nodes, false otherwise.
+   */
   hasChildURIs: function PU_hasChildURIs(aNode) {
     if (!this.nodeIsContainer(aNode))
       return false;
 
-    // in the Library left pane we use excludeItems
-    if (this.nodeIsFolder(aNode) && asQuery(aNode).queryOptions.excludeItems) {
-      var itemId = PlacesUtils.getConcreteItemId(aNode);
-      var contents = this.getFolderContents(itemId, false, false).root;
-      for (var i = 0; i < contents.childCount; ++i) {
-        var child = contents.getChild(i);
-        if (this.nodeIsURI(child))
-          return true;
-      }
-      return false;
+    var root = this.getContainerNodeWithOptions(aNode, false, true);
+    var oldViewer = root.parentResult.viewer;
+    var wasOpen = root.containerOpen;
+    if (!wasOpen) {
+      root.parentResult.viewer = null;
+      root.containerOpen = true;
     }
 
-    var wasOpen = aNode.containerOpen;
-    if (!wasOpen)
-      aNode.containerOpen = true;
     var found = false;
-    for (var i = 0; i < aNode.childCount && !found; i++) {
-      var child = aNode.getChild(i);
+    for (var i = 0; i < root.childCount && !found; i++) {
+      var child = root.getChild(i);
       if (this.nodeIsURI(child))
         found = true;
     }
-    if (!wasOpen)
-      aNode.containerOpen = false;
+
+    if (!wasOpen) {
+      root.containerOpen = false;
+      root.parentResult.viewer = oldViewer;
+    }
     return found;
   },
 
+  /**
+   * Returns an array containing all the uris in the first level of the
+   * passed in container.
+   * If you only need to know if the node contains uris, use hasChildURIs.
+   * @param aNode
+   *        The container node to search through
+   * @returns array of uris in the first level of the container.
+   */
   getURLsForContainerNode: function PU_getURLsForContainerNode(aNode) {
-    let urls = [];
-    if (this.nodeIsFolder(aNode) && asQuery(aNode).queryOptions.excludeItems) {
-      // grab manually
-      var itemId = this.getConcreteItemId(aNode);
-      let contents = this.getFolderContents(itemId, false, false).root;
-      for (let i = 0; i < contents.childCount; ++i) {
-        let child = contents.getChild(i);
-        if (this.nodeIsURI(child))
-          urls.push({uri: child.uri, isBookmark: this.nodeIsBookmark(child)});
-      }
-    }
-    else {
-      let result, oldViewer, wasOpen;
-      try {
-        let wasOpen = aNode.containerOpen;
-        result = aNode.parentResult;
-        oldViewer = result.viewer;
-        if (!wasOpen) {
-          result.viewer = null;
-          aNode.containerOpen = true;
-        }
-        for (let i = 0; i < aNode.childCount; ++i) {
-          // Include visible url nodes only
-          let child = aNode.getChild(i);
-          if (this.nodeIsURI(child)) {
-            // If the node contents is visible, add the uri only if its node is
-            // visible. Otherwise follow viewer's collapseDuplicates property,
-            // default to true
-            if ((wasOpen && oldViewer && child.viewIndex != -1) ||
-                (oldViewer && !oldViewer.collapseDuplicates) ||
-                urls.indexOf(child.uri) == -1) {
-              urls.push({ uri: child.uri,
-                          isBookmark: this.nodeIsBookmark(child) });
-            }
-          }
-        }
-        if (!wasOpen)
-          aNode.containerOpen = false;
-      }
-      finally {
-        if (!wasOpen)
-          result.viewer = oldViewer;
-      }
+    var urls = [];
+    if (!this.nodeIsContainer(aNode))
+      return urls;
+
+    var root = this.getContainerNodeWithOptions(aNode, false, true);
+    var oldViewer = root.parentResult.viewer;
+    var wasOpen = root.containerOpen;
+    if (!wasOpen) {
+      root.parentResult.viewer = null;
+      root.containerOpen = true;
     }
 
+   for (var i = 0; i < root.childCount; ++i) {
+      var child = root.getChild(i);
+      if (this.nodeIsURI(child))
+        urls.push({uri: child.uri, isBookmark: this.nodeIsBookmark(child)});
+    }
+
+    if (!wasOpen) {
+      root.containerOpen = false;
+      root.parentResult.viewer = oldViewer;
+    }
     return urls;
   },
 
   /**
    * Restores bookmarks/tags from a JSON file.
    * WARNING: This method *removes* any bookmarks in the collection before
    * restoring from the file.
    *
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/unit/test_utils_getURLsForContainerNode.js
@@ -0,0 +1,213 @@
+/* -*- 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 Places unit test code.
+ *
+ * The Initial Developer of the Original Code is
+ * Mozilla Corporation.
+ * Portions created by the Initial Developer are Copyright (C) 2009
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *   Marco Bonardo <mak77@bonardo.net> (Original Author)
+ *
+ * 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 ***** */
+
+ /**
+  * Check for correct functionality of PlacesUtils.getURLsForContainerNode and
+  * PlacesUtils.hasChildURIs (those helpers share almost all of their code)
+  */
+
+Components.utils.import("resource://gre/modules/utils.js");
+var PU = PlacesUtils;
+var hs = PU.history;
+var bs = PU.bookmarks;
+
+var tests = [
+
+function() {
+  dump("\n\n*** TEST: folder\n");
+  // This is the folder we will check for children.
+  var folderId = bs.createFolder(bs.toolbarFolder, "folder", bs.DEFAULT_INDEX);
+
+  // Create a folder and a query node inside it, these should not be considered
+  // uri nodes.
+  bs.createFolder(folderId, "inside folder", bs.DEFAULT_INDEX);
+  bs.insertBookmark(folderId, uri("place:sort=1"),
+                    bs.DEFAULT_INDEX, "inside query");
+
+  var query = hs.getNewQuery();
+  query.setFolders([bs.toolbarFolder], 1);
+  var options = hs.getNewQueryOptions();
+
+  dump("Check folder without uri nodes\n");
+  check_uri_nodes(query, options, 0);
+
+  dump("Check folder with uri nodes\n");
+  // Add an uri node, this should be considered.
+  bs.insertBookmark(folderId, uri("http://www.mozilla.org/"),
+                    bs.DEFAULT_INDEX, "bookmark");
+  check_uri_nodes(query, options, 1);
+},
+
+function() {
+  dump("\n\n*** TEST: folder in an excludeItems root\n");
+  // This is the folder we will check for children.
+  var folderId = bs.createFolder(bs.toolbarFolder, "folder", bs.DEFAULT_INDEX);
+
+  // Create a folder and a query node inside it, these should not be considered
+  // uri nodes.
+  bs.createFolder(folderId, "inside folder", bs.DEFAULT_INDEX);
+  bs.insertBookmark(folderId, uri("place:sort=1"), bs.DEFAULT_INDEX, "inside query");
+
+  var query = hs.getNewQuery();
+  query.setFolders([bs.toolbarFolder], 1);
+  var options = hs.getNewQueryOptions();
+  options.excludeItems = true;
+
+  dump("Check folder without uri nodes\n");
+  check_uri_nodes(query, options, 0);
+
+  dump("Check folder with uri nodes\n");
+  // Add an uri node, this should be considered.
+  bs.insertBookmark(folderId, uri("http://www.mozilla.org/"),
+                    bs.DEFAULT_INDEX, "bookmark");
+  check_uri_nodes(query, options, 1);
+},
+
+function() {
+  dump("\n\n*** TEST: query\n");
+  // This is the query we will check for children.
+  bs.insertBookmark(bs.toolbarFolder, uri("place:folder=BOOKMARKS_MENU&sort=1"),
+                    bs.DEFAULT_INDEX, "inside query");
+
+  // Create a folder and a query node inside it, these should not be considered
+  // uri nodes.
+  bs.createFolder(bs.bookmarksMenuFolder, "inside folder", bs.DEFAULT_INDEX);
+  bs.insertBookmark(bs.bookmarksMenuFolder, uri("place:sort=1"),
+                    bs.DEFAULT_INDEX, "inside query");
+
+  var query = hs.getNewQuery();
+  query.setFolders([bs.toolbarFolder], 1);
+  var options = hs.getNewQueryOptions();
+
+  dump("Check query without uri nodes\n");
+  check_uri_nodes(query, options, 0);
+
+  dump("Check query with uri nodes\n");
+  // Add an uri node, this should be considered.
+  bs.insertBookmark(bs.bookmarksMenuFolder, uri("http://www.mozilla.org/"),
+                    bs.DEFAULT_INDEX, "bookmark");
+  check_uri_nodes(query, options, 1);
+},
+
+function() {
+  dump("\n\n*** TEST: excludeItems Query\n");
+  // This is the query we will check for children.
+  bs.insertBookmark(bs.toolbarFolder, uri("place:folder=BOOKMARKS_MENU&sort=8"),
+                    bs.DEFAULT_INDEX, "inside query");
+
+  // Create a folder and a query node inside it, these should not be considered
+  // uri nodes.
+  bs.createFolder(bs.bookmarksMenuFolder, "inside folder", bs.DEFAULT_INDEX);
+  bs.insertBookmark(bs.bookmarksMenuFolder, uri("place:sort=1"),
+                    bs.DEFAULT_INDEX, "inside query");
+
+  var query = hs.getNewQuery();
+  query.setFolders([bs.toolbarFolder], 1);
+  var options = hs.getNewQueryOptions();
+  options.excludeItems = true;
+
+  dump("Check folder without uri nodes\n");
+  check_uri_nodes(query, options, 0);
+
+  dump("Check folder with uri nodes\n");
+  // Add an uri node, this should be considered.
+  bs.insertBookmark(bs.bookmarksMenuFolder, uri("http://www.mozilla.org/"),
+                    bs.DEFAULT_INDEX, "bookmark");
+  check_uri_nodes(query, options, 1);
+},
+
+function() {
+  dump("\n\n*** TEST: !expandQueries Query\n");
+  // This is the query we will check for children.
+  bs.insertBookmark(bs.toolbarFolder, uri("place:folder=BOOKMARKS_MENU&sort=8"),
+                    bs.DEFAULT_INDEX, "inside query");
+
+  // Create a folder and a query node inside it, these should not be considered
+  // uri nodes.
+  bs.createFolder(bs.bookmarksMenuFolder, "inside folder", bs.DEFAULT_INDEX);
+  bs.insertBookmark(bs.bookmarksMenuFolder, uri("place:sort=1"),
+                    bs.DEFAULT_INDEX, "inside query");
+
+  var query = hs.getNewQuery();
+  query.setFolders([bs.toolbarFolder], 1);
+  var options = hs.getNewQueryOptions();
+  options.expandQueries = false;
+
+  dump("Check folder without uri nodes\n");
+  check_uri_nodes(query, options, 0);
+
+  dump("Check folder with uri nodes\n");
+  // Add an uri node, this should be considered.
+  bs.insertBookmark(bs.bookmarksMenuFolder, uri("http://www.mozilla.org/"),
+                    bs.DEFAULT_INDEX, "bookmark");
+  check_uri_nodes(query, options, 1);
+}
+
+];
+
+/**
+ * Executes a query and checks number of uri nodes in the first container in 
+ * query's results.  To correctly test a container ensure that the query will
+ * return only your container in the first level.
+ *
+ * @param  aQuery
+ *         nsINavHistoryQuery object defining the query
+ * @param  aOptions
+ *         nsINavHistoryQueryOptions object defining the query's options
+ * @param  aExpectedURINodes
+ *         number of expected uri nodes
+ */
+function check_uri_nodes(aQuery, aOptions, aExpectedURINodes) {
+  var result = hs.executeQuery(aQuery, aOptions);
+  var root = result.root;
+  root.containerOpen = true;
+  var node = root.getChild(0);
+  do_check_eq(PU.hasChildURIs(node), aExpectedURINodes > 0);
+  do_check_eq(PU.getURLsForContainerNode(node).length, aExpectedURINodes);
+  root.containerOpen = false;
+}
+
+function run_test() {
+  tests.forEach(function(aTest) {
+                  remove_all_bookmarks();
+                  aTest();
+                });
+  // Cleanup.
+  remove_all_bookmarks();
+}