Bug 528044 - Global search: "Show/Open as list" results view does not remember columns; r=mkmelin
☠☠ backed out by cb2e8c1d0782 ☠ ☠
authorJim Porter <jporter@mozilla.com>
Sat, 18 Jan 2014 14:16:27 -0600
changeset 19336 dcc265385f91a1347d8cd4e2a868b174d08cad13
parent 19335 507cf80a989eeb205649c7d4db7bad995ebe52d4
child 19337 7586a9a93b6a2499d03259322b922da8b768c78d
push id1103
push usermbanner@mozilla.com
push dateTue, 18 Mar 2014 07:44:06 +0000
treeherdercomm-beta@50c6279a0af0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmkmelin
bugs528044
Bug 528044 - Global search: "Show/Open as list" results view does not remember columns; r=mkmelin
mail/base/content/folderDisplay.js
mail/base/content/mailTabs.js
mail/base/modules/dbViewWrapper.js
mail/test/mozmill/folder-display/test-columns.js
mailnews/db/gloda/modules/dbview.js
--- a/mail/base/content/folderDisplay.js
+++ b/mail/base/content/folderDisplay.js
@@ -443,18 +443,26 @@ FolderDisplayWidget.prototype = {
    *  column state has been changed.  This could be because there was no
    *  persisted state so we figured out a default one and want to save it.
    *  Otherwise this should be because the user explicitly changed up the column
    *  configurations.  You should not call this willy-nilly.
    *
    * @param aState State to persist.
    */
   _persistColumnStates: function FolderDisplayWidget__persistColumnStates(aState) {
+    if (this.view.isSynthetic) {
+      let syntheticView = this.view._syntheticView;
+      if ("setPersistedSetting" in syntheticView)
+        syntheticView.setPersistedSetting("columns", aState);
+      return;
+    }
+
     if (!this.view.displayedFolder || !this.view.displayedFolder.msgDatabase)
       return;
+
     let msgDatabase = this.view.displayedFolder.msgDatabase;
     let dbFolderInfo = msgDatabase.dBFolderInfo;
     dbFolderInfo.setCharProperty(this.PERSISTED_COLUMN_PROPERTY_NAME,
                                  JSON.stringify(aState));
     msgDatabase.Commit(Components.interfaces.nsMsgDBCommitType.kLargeCommit);
   },
 
   /**
@@ -473,16 +481,23 @@ FolderDisplayWidget.prototype = {
   /**
    * Either inherit the column state of another folder or use heuristics to
    *  figure out the best column state for the current folder.
    */
   _getDefaultColumnsForCurrentFolder:
       function FolderDisplayWidget__getDefaultColumnsForCurrentFolder() {
     const InboxFlag = Components.interfaces.nsMsgFolderFlags.Inbox;
 
+    // If the view is synthetic, try asking it for the default columns.
+    // Otherwise, just go through the usual process.
+    if (this.view.isSynthetic &&
+        "getDefaultSetting" in this.view._syntheticView) {
+      return this.view._syntheticView.getDefaultSetting("columns");
+    }
+
     // do not inherit from the inbox if:
     // - It's an outgoing folder; these have a different use-case and there
     //    should be a small number of these, so it's okay to have no defaults.
     // - It's a virtual folder (single or multi-folder backed).  Who knows what
     //    the intent of the user is in this case.  This should also be bounded
     //    in number and our default heuristics should be pretty good.
     // - News folders.  There is no inbox so there's nothing to inherit from.
     //    (Although we could try and see if they have opened any other news
@@ -951,27 +966,35 @@ FolderDisplayWidget.prototype = {
   },
 
   /**
    * We are entering the folder for display:
    * - set the header cache size.
    * - Setup the columns if we did not already depersist in |onLoadingFolder|.
    */
   onDisplayingFolder: function FolderDisplayWidget_onDisplayingFolder() {
-    let msgDatabase = this.view.displayedFolder.msgDatabase;
+    let displayedFolder = this.view.displayedFolder;
+    let msgDatabase = displayedFolder && displayedFolder.msgDatabase;
     if (msgDatabase) {
       msgDatabase.resetHdrCacheSize(this.PERF_HEADER_CACHE_SIZE);
     }
 
     // makeActive will restore the folder state
     if (!this._savedColumnStates) {
-      // get the default for this folder
-      this._savedColumnStates = this._getDefaultColumnsForCurrentFolder();
-      // and save it so it doesn't wiggle if the inbox/prototype changes
-      this._persistColumnStates(this._savedColumnStates);
+      if (this.view.isSynthetic &&
+          "getPersistedSetting" in this.view._syntheticView) {
+        let columns = this.view._syntheticView.getPersistedSetting("columns");
+        this._savedColumnStates = columns;
+      }
+      else {
+        // get the default for this folder
+        this._savedColumnStates = this._getDefaultColumnsForCurrentFolder();
+        // and save it so it doesn't wiggle if the inbox/prototype changes
+        this._persistColumnStates(this._savedColumnStates);
+      }
     }
 
     FolderDisplayListenerManager._fireListeners("onDisplayingFolder",
                                                 [this]);
 
     if (this.active)
       this.makeActive();
   },
--- a/mail/base/content/mailTabs.js
+++ b/mail/base/content/mailTabs.js
@@ -372,37 +372,16 @@ let mailTabType = {
       type: "glodaSearch",
       /// The set of panes that are legal to be displayed in this mode
       legalPanes: {
         folder: false,
         thread: true,
         message: true,
       },
       /**
-       * The default set of columns to show.  This really should just be for
-       *  boot-strapping and should be persisted after that...
-       */
-      desiredColumnStates: {
-        threadCol: {
-          visible: true,
-        },
-        flaggedCol: {
-          visible: true,
-        },
-        subjectCol: {
-          visible: true,
-        },
-        senderCol: {
-          visible: true,
-        },
-        dateCol: {
-          visible: true,
-        },
-      },
-      /**
        * Open a new folder-display-style tab showing the contents of a gloda
        *  query/collection.  You must pass one of 'query'/'collection'/
        *  'conversation'
        *
        * @param {GlodaQuery} [aArgs.query] An un-triggered gloda query to use.
        *     Alternatively, if you already have a collection, you can pass that
        *     instead as 'collection'.
        * @param {GlodaCollection} [aArgs.collection] A gloda collection to
@@ -418,18 +397,17 @@ let mailTabType = {
        * XXX This needs to handle opening in the background
        */
       openTab: function(aTab, aArgs) {
         aTab.glodaSynView = new GlodaSyntheticView(aArgs);
         aTab.title = aArgs.title;
 
         this.openTab(aTab, false, new MessagePaneDisplayWidget(), false);
         aTab.folderDisplay.show(aTab.glodaSynView);
-        // XXX persist column states in preferences or session store or other
-        aTab.folderDisplay.setColumnStates(aTab.mode.desiredColumnStates);
+        // XXX persist threaded state?
         aTab.folderDisplay.view.showThreaded = true;
 
         let background = ("background" in aArgs) && aArgs.background;
         if (!background)
           aTab.folderDisplay.makeActive();
         if ("message" in aArgs) {
           let hdr = aArgs.message.folderMessage;
           if (hdr)
--- a/mail/base/modules/dbViewWrapper.js
+++ b/mail/base/modules/dbViewWrapper.js
@@ -807,16 +807,17 @@ DBViewWrapper.prototype = {
     this._underlyingData = this.kUnderlyingSynthetic;
     this._syntheticView = aSyntheticView;
 
     this.search = new SearchSpec(this);
     this._sort = this._syntheticView.defaultSort.concat();
 
     this._applyViewChanges();
     FolderNotificationHelper.noteCuriosity(this);
+    this.listener.onDisplayingFolder();
   },
 
   /**
    * Makes us irrevocavbly be a search view, for use in search windows.
    *  Once you call this, you are not allowed to use us for anything
    *  but a search view!
    * We add a 'searchFolders' property that allows you to control what
    *  folders we are searching over.
--- a/mail/test/mozmill/folder-display/test-columns.js
+++ b/mail/test/mozmill/folder-display/test-columns.js
@@ -18,16 +18,18 @@ Cu.import("resource:///modules/MailUtils
 var MODULE_NAME = 'test-columns';
 
 var RELATIVE_ROOT = '../shared-modules';
 var MODULE_REQUIRES = ['folder-display-helpers', 'window-helpers'];
 
 var folderInbox, folderSent, folderVirtual, folderA, folderB;
 // INBOX_DEFAULTS sans 'dateCol' but gains 'tagsCol'
 var columnsB;
+// GLODA_DEFAULTS sans 'locationCol' but gains 'accountCol'
+var glodaColumns;
 
 // these are for the reset/apply to other/apply to other+child tests.
 var folderSource, folderParent, folderChild1, folderChild2;
 
 function setupModule(module) {
   let fdh = collector.getModule('folder-display-helpers');
   fdh.installInto(module);
   let wh = collector.getModule('window-helpers');
@@ -91,17 +93,17 @@ function hide_column(aColumnId) {
  *     before.
  */
 function reorder_column(aColumnId, aBeforeId) {
   let col = mc.e(aColumnId);
   let before = mc.e(aBeforeId);
   mc.threadTree._reorderColumn(col, before, true);
 }
 
-var INBOX_DEFAULTS = [
+const INBOX_DEFAULTS = [
   "threadCol",
   "flaggedCol",
   "attachmentCol",
   "subjectCol",
   "unreadButtonColHeader",
   "senderCol",
   "junkStatusCol",
   "dateCol"
@@ -112,17 +114,17 @@ var INBOX_DEFAULTS = [
  */
 function test_column_defaults_inbox() {
   // just use the inbox; comes from test-folder-display-helpers
   folderInbox = inboxFolder;
   enter_folder(folderInbox);
   assert_visible_columns(INBOX_DEFAULTS);
 }
 
-var SENT_DEFAULTS = [
+const SENT_DEFAULTS = [
   "threadCol",
   "flaggedCol",
   "attachmentCol",
   "subjectCol",
   "unreadButtonColHeader",
   "recipientCol",
   "junkStatusCol",
   "dateCol"
@@ -134,17 +136,17 @@ var SENT_DEFAULTS = [
 function test_column_defaults_sent() {
   folderSent = create_folder("ColumnsSent");
   folderSent.setFlag(Ci.nsMsgFolderFlags.SentMail);
 
   be_in_folder(folderSent);
   assert_visible_columns(SENT_DEFAULTS);
 }
 
-var VIRTUAL_DEFAULTS = [
+const VIRTUAL_DEFAULTS = [
   "threadCol",
   "flaggedCol",
   "attachmentCol",
   "subjectCol",
   "unreadButtonColHeader",
   "senderCol",
   "junkStatusCol",
   "dateCol",
@@ -441,8 +443,71 @@ function test_apply_to_folder_and_childr
   be_in_folder(folderParent);
   assert_visible_columns(conExtra);
   be_in_folder(folderChild1);
   assert_visible_columns(conExtra);
   be_in_folder(folderChild2);
   assert_visible_columns(conExtra);
 }
 test_apply_to_folder_and_children.EXCLUDED_PLATFORMS = ["linux"];
+
+const GLODA_DEFAULTS = [
+  "threadCol",
+  "flaggedCol",
+  "subjectCol",
+  "senderCol",
+  "dateCol",
+  "locationCol"
+];
+
+/**
+ * Create a fake gloda collection.
+ */
+function FakeCollection() {
+  this.items = [];
+}
+
+/**
+ * Make sure that opening a gloda list tab shows the correct columns.
+ */
+function test_column_defaults_gloda_collection() {
+  let fakeCollection = new FakeCollection();
+  mc.tabmail.openTab("glodaList", { collection: fakeCollection });
+  wait_for_all_messages_to_load();
+  assert_visible_columns(GLODA_DEFAULTS);
+}
+
+/**
+ * Open a gloda tab, change the columns, and make sure the changes persist to
+ * subsequent gloda tabs.
+ */
+function test_persist_columns_gloda_collection() {
+  let fakeCollection = new FakeCollection();
+  mc.tabmail.openTab("glodaList", { collection: fakeCollection });
+  wait_for_all_messages_to_load();
+  hide_column("locationCol");
+  show_column("accountCol");
+
+  glodaColumns = GLODA_DEFAULTS.slice(0, -1);
+  glodaColumns.push("accountCol");
+
+  mc.tabmail.openTab("glodaList", { collection: fakeCollection });
+  wait_for_all_messages_to_load();
+  assert_visible_columns(glodaColumns);
+}
+
+/**
+ * Open a gloda tab and make sure that resetting the columns to the default
+ * works correctly and persists the change to subsequent gloda tabs.
+ */
+function test_reset_columns_gloda_collection() {
+  let fakeCollection = new FakeCollection();
+  mc.tabmail.openTab("glodaList", { collection: fakeCollection });
+  wait_for_all_messages_to_load();
+  assert_visible_columns(glodaColumns);
+
+  invoke_column_picker_option([{anonid: "reset"}]);
+  assert_visible_columns(GLODA_DEFAULTS);
+
+  mc.tabmail.openTab("glodaList", { collection: fakeCollection });
+  wait_for_all_messages_to_load();
+  assert_visible_columns(GLODA_DEFAULTS);
+}
--- a/mailnews/db/gloda/modules/dbview.js
+++ b/mailnews/db/gloda/modules/dbview.js
@@ -9,16 +9,17 @@
 
 const EXPORTED_SYMBOLS = ["GlodaSyntheticView"];
 
 const Cc = Components.classes;
 const Ci = Components.interfaces;
 const Cr = Components.results;
 const Cu = Components.utils;
 
+Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource:///modules/gloda/log4moz.js");
 
 Cu.import("resource:///modules/gloda/public.js");
 Cu.import("resource:///modules/gloda/msg_search.js");
 
 /**
  * Create a synthetic view suitable for passing to |FolderDisplayWidget.show|.
  * You must pass a query, collection, or conversation in.
@@ -30,26 +31,29 @@ Cu.import("resource:///modules/gloda/msg
  * @param {GlodaConversation} [aArgs.conversation] A conversation whose messages
  *     you want to display.
  */
 function GlodaSyntheticView(aArgs) {
   if ("query" in aArgs) {
     this.query = aArgs.query;
     this.collection = this.query.getCollection(this);
     this.completed = false;
+    this.viewType = "global";
   }
   else if ("collection" in aArgs) {
     this.query = null;
     this.collection = aArgs.collection;
     this.completed = true;
+    this.viewType = "global";
   }
   else if ("conversation" in aArgs) {
     this.collection = aArgs.conversation.getMessagesCollection(this);
     this.query = this.collection.query;
     this.completed = false;
+    this.viewType = "conversation";
   }
   else {
     throw new Error("You need to pass a query or collection");
   }
 
   this.customColumns = [];
 }
 GlodaSyntheticView.prototype = {
@@ -100,16 +104,63 @@ GlodaSyntheticView.prototype = {
         let hdr = item.folderMessage;
         if (hdr)
           return hdr;
       }
     }
     return null;
   },
 
+  /**
+   * The default set of columns to show.
+   */
+  DEFAULT_COLUMN_STATES: {
+    threadCol: {
+      visible: true,
+    },
+    flaggedCol: {
+      visible: true,
+    },
+    subjectCol: {
+      visible: true,
+    },
+    senderCol: {
+      visible: true,
+    },
+    dateCol: {
+      visible: true,
+    },
+    locationCol: {
+      visible: true,
+    },
+  },
+
+  // --- settings persistence
+  getPersistedSetting: function(aSetting) {
+    try {
+      return JSON.parse(Services.prefs.getCharPref(
+        "mailnews.database.global.views." + this.viewType + "." + aSetting
+      ));
+    }
+    catch (e) {
+      return this.getDefaultSetting(aSetting);
+    }
+  },
+  setPersistedSetting: function(aSetting, aValue) {
+    Services.prefs.setCharPref(
+      "mailnews.database.global.views." + this.viewType + "." + aSetting,
+      JSON.stringify(aValue)
+    );
+  },
+  getDefaultSetting: function(aSetting) {
+    if (aSetting == "columns")
+      return this.DEFAULT_COLUMN_STATES;
+    return undefined;
+  },
+
   // --- collection listener
   onItemsAdded: function(aItems, aCollection) {
     if (this.searchListener)
       this.reportResults(aItems);
   },
   onItemsModified: function(aItems, aCollection) {
   },
   onItemsRemoved: function(aItems, aCollection) {