Including 318809 - group by site/group by page is not sticky in places. r=annie

browser/components/places/content/controller.js

@@ -102,6 +102,7 @@ function InsertionPoint(folderId, index, orientation) {
 
 /**
  * Initialization Configuration for a View
+ * @constructor
  */
 function ViewConfig(dropTypes, dropOnTypes, filterOptions, firstDropIndex) {
   this.dropTypes = dropTypes;
@@ -114,6 +115,82 @@ ViewConfig.GENERIC_DROP_TYPES = [TYPE_X_MOZ_PLACE_CONTAINER, TYPE_X_MOZ_PLACE,
 ViewConfig.GENERIC_FILTER_OPTIONS = Ci.nsINavHistoryQuery.INCLUDE_ITEMS +
                                      Ci.nsINavHistoryQuery.INCLUDE_QUERIES;
 
+/**
+ * Manages grouping options for a particular view type. 
+ * @param   pref
+ *          The preference that stores these grouping options. 
+ * @param   defaultGroupings
+ *          The default groupings to be used for views of this type. 
+ * @param   serializable
+ *          An object bearing a serialize and deserialize method that
+ *          read and write the object's string representation from/to
+ *          preferences.
+ * @constructor
+ */
+function PrefHandler(pref, defaultValue, serializable) {
+  this._pref = pref;
+  this._defaultValue = defaultValue;
+  this._serializable = serializable;
+
+  this._pb = 
+    Cc["@mozilla.org/preferences-service;1"].
+    getService(Components.interfaces.nsIPrefBranch2);
+  this._pb.addObserver(this._pref, this, false);
+}
+PrefHandler.prototype = {
+  /**
+   * Clean up when the window is going away to avoid leaks. 
+   */
+  destroy: function PC_PH_destroy() {
+    this._pb.removeObserver(this._pref, this);
+  },
+
+  /** 
+   * Observes changes to the preferences.
+   * @param   subject
+   * @param   topic
+   *          The preference changed notification
+   * @param   data
+   *          The preference that changed
+   */
+  observe: function PC_PH_observe(subject, topic, data) {
+    if (topic == "nsPref:changed" && data == this._pref)
+      this._value = null;
+  },
+  
+  /**
+   * The cached value, null if it needs to be rebuilt from preferences.
+   */
+  _value: null,
+
+  /** 
+   * Get the preference value, reading from preferences if necessary. 
+   */
+  get value() { 
+    if (!this._value) {
+      if (this._pb.prefHasUserValue(this._pref)) {
+        var valueString = this._pb.getCharPref(this._pref);
+        this._value = this._serializable.deserialize(valueString);
+      }
+      else
+        this._value = this._defaultValue;
+    }
+    return this._value;
+  },
+  
+  /**
+   * Stores a value in preferences. 
+   * @param   value
+   *          The data to be stored. 
+   */
+  set value(value) {
+    if (value != this._value)
+      this._pb.setCharPref(this._pref, this._serializable.serialize(value));
+    return value;
+  },
+};
+
+
 /**
  * The Master Places Controller
  */
@@ -174,6 +251,20 @@ var PlacesController = {
     return this._hist.executeQuery(query, options);
   },
   
+  /**
+   * Gets a place: URI for the given queries and options. 
+   * @param   queries
+   *          An array of NavHistoryQueries
+   * @param   options
+   *          A NavHistoryQueryOptions object
+   * @returns A place: URI encoding the parameters. 
+   */
+  getPlaceURI: function PC_getPlaceURI(queries, options) {
+    var queryString = this._hist.queriesToQueryString(queries, queries.length,
+                                                      options);
+    return this._uri(queryString);
+  },
+  
   /**
    * The currently active Places view. 
    */
@@ -186,6 +277,18 @@ var PlacesController = {
     return this._activeView;
   },
   
+  /**
+   * The current groupable Places view.
+   */
+  _groupableView: null,
+  get groupableView() {
+    return this._groupableView;
+  },
+  set groupableView(groupableView) {
+    this._groupableView = groupableView;
+    return this._groupableView;
+  },
+  
   isCommandEnabled: function PC_isCommandEnabled(command) {
     //LOG("isCommandEnabled: " + command);
     return document.getElementById(command).getAttribute("disabled") == "true";
@@ -541,28 +644,52 @@ var PlacesController = {
     }
   },
   
+  /**
+   * A hash of groupers that supply grouping options for queries of a given 
+   * type. This is an override of grouping options that might be encoded in
+   * a saved place: URI
+   */
+  groupers: { },
+  
   /**
    * Rebuilds the view using a new set of grouping options.
-   * @param   options
+   * @param   groupings
    *          An array of grouping options, see nsINavHistoryQueryOptions
    *          for details.
    */
-  setGroupingMode: function PC_setGroupingOptions(options) {
+  setGroupingMode: function PC_setGroupingOptions(groupings) {
-    var result = this._activeView.view.QueryInterface(Ci.nsINavHistoryResult);
+    if (!this._groupableView)
+      return;
+    var result = this._groupableView.getResult();
     var queries = result.getQueries({ });
     var newOptions = result.queryOptions.clone();
-    newOptions.setGroupingMode(options, options.length);
     
-    this._activeView.load(queries, newOptions);
+    // Update the grouping mode only after persisting, so that the URI is not 
+    // changed. 
+    newOptions.setGroupingMode(groupings, groupings.length);
+    
+    // Persist this selection
+    if (this._groupableView.isBookmarks && "bookmark" in this.groupers)
+      this.groupers.bookmark.value = groupings;
+    else if ("generic" in this.groupers)
+      this.groupers.generic.value = groupings;
+
+    // Reload the view 
+    this._groupableView.load(queries, newOptions);
   },
   
   /**
    * Group the current content view by domain
    */
   groupBySite: function PC_groupBySite() {
-    var modes = [Ci.nsINavHistoryQueryOptions.GROUP_BY_DOMAIN, 
+    this.setGroupingMode([Ci.nsINavHistoryQueryOptions.GROUP_BY_DOMAIN]);
-    Ci.nsINavHistoryQueryOptions.GROUP_BY_HOST];
+  },
-    this.setGroupingMode(modes);
+  
+  /**
+   * Group the current content view by folder
+   */
+  groupByFolder: function PC_groupByFolder() {
+    this.setGroupingMode([Ci.nsINavHistoryQueryOptions.GROUP_BY_FOLDER]);
   },
   
   /**

browser/components/places/content/places.js

@@ -35,6 +35,9 @@
  *
  * ***** END LICENSE BLOCK ***** */
 
+const PREF_PLACES_GROUPING_GENERIC = "browser.places.grouping.generic";
+const PREF_PLACES_GROUPING_BOOKMARK = "browser.places.grouping.bookmark";
+
 var PlacesUIHook = {
   _tabbrowser: null,
   _topWindow: null,
@@ -94,17 +97,6 @@ var PlacesUIHook = {
   },
 };
 
-function ViewConfig(dropTypes, dropOnTypes, filterOptions, firstDropIndex) {
-  this.dropTypes = dropTypes;
-  this.dropOnTypes = dropOnTypes;
-  this.filterOptions = filterOptions;
-  this.firstDropIndex = firstDropIndex;
-}
-ViewConfig.GENERIC_DROP_TYPES = 
-  [TYPE_X_MOZ_PLACE_CONTAINER, TYPE_X_MOZ_PLACE, TYPE_X_MOZ_URL];
-ViewConfig.GENERIC_FILTER_OPTIONS = 
-  Ci.nsINavHistoryQuery.INCLUDE_ITEMS + Ci.nsINavHistoryQuery.INCLUDE_QUERIES;
-
 var PlacesPage = {
   _content: null,
   _places: null,
@@ -123,6 +115,23 @@ var PlacesPage = {
     this._content.init(new ViewConfig(ViewConfig.GENERIC_DROP_TYPES,
                                       ViewConfig.GENERIC_DROP_TYPES,
                                       ViewConfig.GENERIC_FILTER_OPTIONS, 0));
+                                      
+    PlacesController.groupableView = this._content;
+
+    var GroupingSerializer = {
+      serialize: function GS_serialize(raw) {
+        return raw.join(",");
+      },
+      deserialize: function GS_deserialize(str) {
+        return str === "" ? [] : str.split(",");
+      }
+    };
+    PlacesController.groupers.generic = 
+      new PrefHandler(PREF_PLACES_GROUPING_GENERIC, 
+        [Ci.nsINavHistoryQueryOptions.GROUP_BY_DOMAIN], GroupingSerializer);
+    PlacesController.groupers.bookmark = 
+      new PrefHandler(PREF_PLACES_GROUPING_BOOKMARK,
+        [Ci.nsINavHistoryQueryOptions.GROUP_BY_FOLDER], GroupingSerializer);
     
     // Hook the browser UI
     PlacesUIHook.init(this._content);
@@ -196,7 +205,7 @@ var PlacesPage = {
       break;
     }
   },
-
+  
   /**
    * Called when a place folder is selected in the left pane.
    */
@@ -205,12 +214,21 @@ var PlacesPage = {
     if (!node || this._places.suppressSelection)
       return;
     var queries = node.getQueries({});
-    if (PlacesController.nodeIsFolder(node))
+    var newQueries = [];
-      this._content.loadFolder(node.folderId);
+    for (var i = 0; i < queries.length; ++i) {
-    else { // XXXben, this is risky, need to filter out TYPE_DAY/TYPE_HOST
+      var query = queries[i].clone();
-      var queries = node.getQueries({ });
+      query.itemTypes |= this._content.filterOptions;
-      this._content.load(queries, node.queryOptions);
+      newQueries.push(query);
     }
+    var newOptions = node.queryOptions.clone();
+
+    var groupings = PlacesController.groupers.generic.value;
+    var isBookmark = PlacesController.nodeIsFolder(node);
+    if (isBookmark)
+      groupings = PlacesController.groupers.bookmark.value;
+
+    newOptions.setGroupingMode(groupings, groupings.length);
+    this._content.load(newQueries, newOptions);
   },
   
   /**
@@ -258,7 +276,7 @@ var PlacesPage = {
   onContentChanged: function PP_onContentChanged() {
     var panelID = "commands_history";
     var filterButtonID = "filterList_history";
-    var isBookmarks = PlacesController.nodeIsFolder(this._content.getResult());
+    var isBookmarks = this._content.isBookmarks;
     if (isBookmarks) {
       // if (query.annotation == "feed") {
       panelID = "commands_bookmark";

browser/components/places/content/places.xul

@@ -104,13 +104,17 @@
                 ondblclick="PlacesController.mouseLoadURI(event);"
                 onplacestreereloaded="PlacesPage.onContentChanged();">
             <treecols>
-              <treecol label="&col.title.label;" id="title" flex="5" primary="true"/>
+              <treecol label="&col.title.label;" id="title" flex="5" primary="true" 
+                       persist="width hidden ordinal sortActive sortDirection"/>
               <splitter class="tree-splitter"/>
-              <treecol label="&col.url.label;" id="url" flex="5"/>
+              <treecol label="&col.url.label;" id="url" flex="5" 
+                       persist="width hidden ordinal sortActive sortDirection"/>
               <splitter class="tree-splitter"/>
-              <treecol label="&col.lastvisit.label;" id="date" flex="1"/>
+              <treecol label="&col.lastvisit.label;" id="date" flex="1" 
+                       persist="width hidden ordinal sortActive sortDirection"/>
               <splitter class="tree-splitter"/>
-              <treecol label="&col.visitcount.label;" id="visitCount" flex="1" hidden="true"/>
+              <treecol label="&col.visitcount.label;" id="visitCount" flex="1" hidden="true" 
+                       persist="width hidden ordinal sortActive sortDirection"/>
             </treecols>
             <treechildren id="placeContentChildren" view="placeContent" flex="1"/>
           </tree>

browser/components/places/content/tree.xml

@@ -173,6 +173,12 @@
           this._fireEvent("reloaded");
         ]]></body>
       </method>
+      
+      <property name="isBookmarks">
+        <getter><![CDATA[ 
+          return PlacesController.nodeIsFolder(this.getResult());
+        ]]></getter>
+      </property>
     
       <!-- AVI Method -->
       <property name="hasSelection">