Reland bug 765637 - Selecting multiple tags in quickfilter/view-button-menu should allow AND-operation. ui-r=bwinton, r=asuth
authorMagnus Melin <mkmelin+mozilla@iki.fi>
Thu, 13 Jun 2013 13:50:43 +0300
changeset 15725 1769f50bf805914637e1dd2f5dfb76c1c4a0aa3c
parent 15724 188ea3049dafaea5a029e8093b50ba8f90a7596d
child 15726 47d08a243bdee2260ed54529352a3f41aa62b2d9
push id942
push userbugzilla@standard8.plus.com
push dateMon, 05 Aug 2013 19:15:38 +0000
treeherdercomm-beta@0e1a1c4a9f0c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbwinton, asuth
bugs765637
Reland bug 765637 - Selecting multiple tags in quickfilter/view-button-menu should allow AND-operation. ui-r=bwinton, r=asuth
mail/base/content/folderDisplay.js
mail/base/content/quickFilterBar.js
mail/base/content/quickFilterBar.xul
mail/base/modules/quickFilterManager.js
mail/test/mozmill/quick-filter-bar/test-filter-logic.js
mail/test/mozmill/quick-filter-bar/test-keyboard-interface.js
mail/test/mozmill/shared-modules/test-quick-filter-bar-helper.js
--- a/mail/base/content/folderDisplay.js
+++ b/mail/base/content/folderDisplay.js
@@ -61,18 +61,19 @@ let FolderDisplayListenerManager = {
    * For use by FolderDisplayWidget to trigger listener invocation.
    */
   _fireListeners: function FDBLM__fireListeners(aEventName, aArgs) {
     for each (let [, listener] in Iterator(this._listeners)) {
       if (aEventName in listener) {
         try {
           listener[aEventName].apply(listener, aArgs);
         }
-        catch(ex) {
-          Components.utils.reportError(ex);
+        catch(e) {
+          Components.utils.reportError(aEventName + " event listener FAILED; " +
+                                       e + " at: " + e.stack);
         }
       }
     }
   },
 };
 
 /**
  * Abstraction for a widget that (roughly speaking) displays the contents of
--- a/mail/base/content/quickFilterBar.js
+++ b/mail/base/content/quickFilterBar.js
@@ -510,23 +510,25 @@ let QuickFilterBarMuxer = {
     else if (aCommand == "cmd_toggleQuickFilterBar") {
       this._showFilterBar(!this.activeFilterer.visible);
       return true;
     }
     return null;
   },
 
   get maybeActiveFilterer() {
-    if ("quickFilter" in this.tabmail.currentTabInfo._ext)
+    if (this.tabmail.currentTabInfo &&
+       "quickFilter" in this.tabmail.currentTabInfo._ext)
       return this.tabmail.currentTabInfo._ext.quickFilter;
     return null;
   },
 
   get activeFilterer() {
-    if ("quickFilter" in this.tabmail.currentTabInfo._ext)
+    if (this.tabmail.currentTabInfo &&
+        "quickFilter" in this.tabmail.currentTabInfo._ext)
       return this.tabmail.currentTabInfo._ext.quickFilter;
     throw errorWithDebug("There is no active filterer but we want one.");
   },
 
   //////////////////////////////////////////////////////////////////////////////
   // Event Handling Support
 
   /**
--- a/mail/base/content/quickFilterBar.xul
+++ b/mail/base/content/quickFilterBar.xul
@@ -145,16 +145,28 @@
                  minwidth="280">
         </textbox>
       </hbox>
       <hbox id="quick-filter-bar-expando">
         <arrowscrollbox id="quick-filter-bar-tab-bar"
                         orient="horizontal"
                         collapsed="true"
                         flex="2">
+        <menulist id="qfb-boolean-mode"
+                  tooltiptext="&quickFilterBar.booleanMode.tooltip;"
+                  persist="value">
+          <menupopup id="qfb-boolean-mode-popup">
+            <menuitem id="qfb-boolean-mode-or" value="OR"
+                      label="&quickFilterBar.booleanModeAny.label;"
+                      tooltiptext="&quickFilterBar.booleanModeAny.tooltip;"/>
+            <menuitem id="qfb-boolean-mode-and" value="AND"
+                      label="&quickFilterBar.booleanModeAll.label;"
+                      tooltiptext="&quickFilterBar.booleanModeAll.tooltip;"/>
+          </menupopup>
+        </menulist>
         </arrowscrollbox>
         <hbox id="quick-filter-bar-filter-text-bar"
               collapsed="true"
               pack="end"
               align="center"
               flex="1">
           <label id="qfb-qs-label"
                  value="&quickFilterBar.textFilter.explanation.label;"/>
--- a/mail/base/modules/quickFilterManager.js
+++ b/mail/base/modules/quickFilterManager.js
@@ -634,39 +634,40 @@ let TagFacetingFilter = {
   /**
    * @return true if the constaint is only on has tags/does not have tags,
    *     false if there are specific tag constraints in play.
    */
   isSimple: function(aFilterValue) {
     // it's the simple case if the value is just a boolean
     if (typeof(aFilterValue) != "object")
       return true;
-    // but also if the object contains no true values
+    // but also if the object contains no non-null values
     let simpleCase = true;
-    for each (let [key, value] in Iterator(aFilterValue)) {
+    for each (let [key, value] in Iterator(aFilterValue.tags)) {
       if (value !== null) {
         simpleCase = false;
         break;
       }
     }
     return simpleCase;
   },
 
   /**
    * Because we support both inclusion and exclusion we can produce up to two
    *  groups.  One group for inclusion, one group for exclusion.  To get listed
-   *  you only need to include one of the tags marked for inclusion, but you
-   *  must not have any of the tags marked for exclusion.
+   *  the message must have any/all of the tags marked for inclusion,
+   *  (depending on mode), but it cannot have any of the tags marked for
+   *  exclusion.
    */
   appendTerms: function TFF_appendTerms(aTermCreator, aTerms, aFilterValue) {
-    let term, value;
-
     if (aFilterValue == null)
       return null;
 
+    let term, value;
+
     // just the true/false case
     if (this.isSimple(aFilterValue)) {
       term = aTermCreator.createTerm();
       term.attrib = Components.interfaces.nsMsgSearchAttrib.Keywords;
       value = term.value;
       value.str = "";
       term.value = value;
       term.op = aFilterValue ?
@@ -681,28 +682,30 @@ let TagFacetingFilter = {
     }
     else {
       let firstIncludeClause = true, firstExcludeClause = true;
       let lastIncludeTerm = null;
       term = null;
 
       let excludeTerms = [];
 
-      for each (let [key, shouldFilter] in Iterator(aFilterValue)) {
+      let mode = aFilterValue.mode;
+      for each (let [key, shouldFilter] in Iterator(aFilterValue.tags)) {
         if (shouldFilter !== null) {
           term = aTermCreator.createTerm();
           term.attrib = Components.interfaces.nsMsgSearchAttrib.Keywords;
           value = term.value;
           value.attrib = term.attrib;
           value.str = key;
           term.value = value;
           if (shouldFilter) {
             term.op = nsMsgSearchOp.Contains;
-            // AND for the group, but OR inside the group
-            term.booleanAnd = firstIncludeClause;
+            // AND for the group. Inside the group we also want AND if the
+            // mode is set to "All of".
+            term.booleanAnd = firstIncludeClause || (mode === "AND");
             term.beginsGrouping = firstIncludeClause;
             aTerms.push(term);
             firstIncludeClause = false;
             lastIncludeTerm = term;
           }
           else {
             term.op = nsMsgSearchOp.DoesntContain;
             // you need to not include all of the tags marked excluded.
@@ -733,17 +736,16 @@ let TagFacetingFilter = {
           aTerms.push(term);
         }
 
         // (extend in the exclusions)
         excludeTerms[excludeTerms.length-1].endsGrouping = true;
         aTerms.push.apply(aTerms, excludeTerms);
       }
     }
-
     return null;
   },
 
   onSearchStart: function(aCurState) {
     // this becomes aKeywordMap; we want to start with an empty one
     return {};
   },
   onSearchMessage: function(aKeywordMap, aMsgHdr, aFolder) {
@@ -756,26 +758,25 @@ let TagFacetingFilter = {
   },
   onSearchDone: function(aCurState, aKeywordMap, aStatus) {
     // we are an async operation; if the user turned off the tag facet already,
     //  then leave that state intact...
     if (aCurState == null)
       return [null, false, false];
 
     // only propagate things that are actually tags though!
-    let outKeyMap = {};
+    let outKeyMap = {tags: {}};
     let tags = MailServices.tags.getAllTags({});
     let tagCount = tags.length;
     for (let iTag = 0; iTag < tagCount; iTag++) {
       let tag = tags[iTag];
 
       if (tag.key in aKeywordMap)
-        outKeyMap[tag.key] = aKeywordMap[tag.key];
+        outKeyMap.tags[tag.key] = aKeywordMap[tag.key];
     }
-
     return [outKeyMap, true, false];
   },
 
   /**
    * We need to clone our state if it's an object to avoid bad sharing.
    */
   propagateState: function(aOld, aSticky) {
     // stay disabled when disabled, get disabled when not sticky
@@ -796,62 +797,82 @@ let TagFacetingFilter = {
     if (!checked)
       aDocument.getElementById("quick-filter-bar-tab-bar").collapsed = true;
 
     // return ourselves if we just got checked to have
     //  onSearchStart/onSearchMessage/onSearchDone get to do their thing.
     return [checked, true];
   },
 
+  domBindExtra: function(aDocument, aMuxer, aNode) {
+    // Tag filtering mode menu (All of/Any of)
+    function commandHandler(aEvent) {
+      let filterValue = aMuxer.getFilterValueForMutation(TagFacetingFilter.name);
+      filterValue.mode = aEvent.target.value;
+      aMuxer.updateSearch();
+    }
+    aDocument.getElementById("qfb-boolean-mode").addEventListener(
+      "ValueChange", commandHandler, false);
+  },
+
   reflectInDOM: function TFF_reflectInDOM(aNode, aFilterValue,
                                           aDocument, aMuxer) {
     aNode.checked = aFilterValue ? true : false;
-
     if ((aFilterValue != null) &&
         (typeof(aFilterValue) == "object"))
       this._populateTagBar(aFilterValue, aDocument, aMuxer);
     else
       aDocument.getElementById("quick-filter-bar-tab-bar").collapsed = true;
   },
 
   _populateTagBar: function TFF__populateTagMenu(aState, aDocument, aMuxer) {
     let tagbar = aDocument.getElementById("quick-filter-bar-tab-bar");
-    let keywordMap = aState;
+    let keywordMap = aState.tags;
+
+    // If we have a mode stored use that. If we don't have a mode, then update
+    // our state to agree with what the UI is currently displaying;
+    // this will happen for fresh profiles.
+    let qbm = aDocument.getElementById("qfb-boolean-mode");
+    if (aState.mode)
+      qbm.value = aState.mode;
+    else
+      aState.mode = qbm.value;
 
     function commandHandler(aEvent) {
       let tagKey = aEvent.target.getAttribute("value");
       let state = aMuxer.getFilterValueForMutation(TagFacetingFilter.name);
-      state[tagKey] = aEvent.target.checked ? true : null;
+      state.tags[tagKey] = aEvent.target.checked ? true : null;
       aEvent.target.removeAttribute("inverted");
       aMuxer.updateSearch();
     };
 
     function rightClickHandler(aEvent) {
       // Only do something if this is a right-click, otherwise commandHandler
       //  will pick up on it.
       if (aEvent.button == 2) {
         // we need to toggle the checked state ourselves
         aEvent.target.checked = !aEvent.target.checked;
 
         let tagKey = aEvent.target.getAttribute("value");
         let state = aMuxer.getFilterValueForMutation(TagFacetingFilter.name);
-        state[tagKey] = aEvent.target.checked ? false : null;
+        state.tags[tagKey] = aEvent.target.checked ? false : null;
         if (aEvent.target.checked)
           aEvent.target.setAttribute("inverted", "true");
         else
           aEvent.target.removeAttribute("inverted");
         aMuxer.updateSearch();
         aEvent.stopPropagation();
         aEvent.preventDefault();
       }
     }
 
-    // -- nuke existing exposed tags
-    while (tagbar.lastChild)
+    // -- nuke existing exposed tags, but not the mode selector (which is first)
+    while (tagbar.lastChild && tagbar.lastChild !== tagbar.firstChild) {
       tagbar.removeChild(tagbar.lastChild);
+    }
 
     let addCount = 0;
 
     // -- create an element for each tag
     let tags = MailServices.tags.getAllTags({});
     let tagCount = tags.length;
     for (let iTag = 0; iTag < tagCount; iTag++) {
       let tag = tags[iTag];
@@ -879,17 +900,16 @@ let TagFacetingFilter = {
         // everybody always gets to be an qfb-tag-button.
         if (color)
           button.setAttribute("class", "qfb-tag-button lc-" + color.substr(1));
         else
           button.setAttribute("class", "qfb-tag-button");
         tagbar.appendChild(button);
       }
     }
-
     tagbar.collapsed = !addCount;
   },
 };
 QuickFilterManager.defineFilter(TagFacetingFilter);
 
 /**
  * true: must have attachment, false: must not have attachment.
  */
--- a/mail/test/mozmill/quick-filter-bar/test-filter-logic.js
+++ b/mail/test/mozmill/quick-filter-bar/test-filter-logic.js
@@ -1,31 +1,30 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-/*
+/**
  * Verify that we are constructing the filters that we expect and that they
  * are hooked up to the right buttons.
  */
 
+// make SOLO_TEST=quick-filter-bar/test-filter-logic.js mozmill-one
+
 var MODULE_NAME = 'test-filter-logic';
 
 const RELATIVE_ROOT = '../shared-modules';
 
 var MODULE_REQUIRES = ['folder-display-helpers', 'window-helpers',
                        'quick-filter-bar-helper'];
 
 function setupModule(module) {
-  let fdh = collector.getModule('folder-display-helpers');
-  fdh.installInto(module);
-  let wh = collector.getModule('window-helpers');
-  wh.installInto(module);
-  let qfb = collector.getModule('quick-filter-bar-helper');
-  qfb.installInto(module);
+  collector.getModule("folder-display-helpers").installInto(module);
+  collector.getModule("window-helpers").installInto(module);
+  collector.getModule("quick-filter-bar-helper").installInto(module);
 }
 
 function test_filter_unread() {
   let folder = create_folder("QuickFilterBarFilterUnread");
   let [unread, read] = make_new_sets_in_folder(folder,
     [{count: 1}, {count: 1}]);
   read.setRead(true);
 
@@ -134,19 +133,24 @@ function test_filter_tags() {
 
   be_in_folder(folder);
   toggle_boolean_constraints("tags"); // must have a tag
   assert_messages_in_view([setTagA, setTagB, setTagAB, setTagC]);
 
   toggle_tag_constraints(tagA); // must have tag A
   assert_messages_in_view([setTagA, setTagAB]);
 
-  toggle_tag_constraints(tagB); // must have tag A OR tag B
+  toggle_tag_constraints(tagB);
+  // mode is OR by default -> must have tag A or tag B
   assert_messages_in_view([setTagA, setTagB, setTagAB]);
 
+  toggle_tag_mode();
+  // mode is now AND -> must have tag A and tag B
+  assert_messages_in_view([setTagAB]);
+
   toggle_tag_constraints(tagA); // must have tag B
   assert_messages_in_view([setTagB, setTagAB]);
 
   toggle_tag_constraints(tagB); // have have a tag
   assert_messages_in_view([setTagA, setTagB, setTagAB, setTagC]);
 
   toggle_boolean_constraints("tags"); // no constraints
   assert_messages_in_view([setNoTag, setTagA, setTagB, setTagAB, setTagC]);
--- a/mail/test/mozmill/quick-filter-bar/test-keyboard-interface.js
+++ b/mail/test/mozmill/quick-filter-bar/test-keyboard-interface.js
@@ -1,13 +1,13 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-/*
+/**
  * Tests keyboard stuff that doesn't fall under some other test's heading.
  * Namely, control-shift-k toggling the bar into existence happens in
  * test-toggle-bar.js, but we test that repeatedly hitting control-shift-k
  * selects the text entered in the quick filter bar.
  */
 
 var MODULE_NAME = 'test-keyboard-interface';
 
@@ -113,17 +113,16 @@ function test_escape_does_not_reach_us_f
 
     mc.keypress(null, "VK_ESCAPE", {});
 
     assert_quick_filter_bar_visible(true);
   }
   finally {
     glodaSearchWidget.collapsed = true;
   }
-
 }
 
 /**
  * Control-shift-k expands the quick filter bar when it's collapsed. When
  * already expanded, it focuses the text box and selects its text.
  */
 function test_control_shift_k_shows_quick_filter_bar() {
   let dispatcha = mc.window.document.commandDispatcher;
--- a/mail/test/mozmill/shared-modules/test-quick-filter-bar-helper.js
+++ b/mail/test/mozmill/shared-modules/test-quick-filter-bar-helper.js
@@ -21,16 +21,17 @@ function setupModule(module) {
 
 var EXPORT = [
   'assert_quick_filter_button_enabled',
   'assert_quick_filter_bar_visible',
   'toggle_quick_filter_bar',
   'assert_constraints_expressed',
   'toggle_boolean_constraints',
   'toggle_tag_constraints',
+  'toggle_tag_mode',
   'assert_tag_constraints_visible',
   'assert_tag_constraints_checked',
   'toggle_text_constraints',
   'assert_text_constraints_checked',
   'set_filter_text',
   'assert_filter_text',
   'assert_results_label_count',
   'clear_constraints',
@@ -125,35 +126,55 @@ function toggle_boolean_constraints() {
 function toggle_tag_constraints() {
   for (let iArg = 0; iArg < arguments.length; iArg++) {
     mc.click(mc.eid("qfb-tag-" + arguments[iArg]));
   }
   wait_for_all_messages_to_load(mc);
 }
 
 /**
+ * Set the tag filtering mode. Wait for messages after.
+ */
+function toggle_tag_mode() {
+  let qbm = mc.e("qfb-boolean-mode");
+  if (qbm.value === "AND") {
+    qbm.selectedIndex--; // = move to "OR";
+    assert_equals(qbm.value, "OR", "qfb-boolean-mode has wrong state");
+  }
+  else if (qbm.value === "OR") {
+    qbm.selectedIndex++; // = move to "AND";
+    assert_equals(qbm.value, "AND", "qfb-boolean-mode has wrong state");
+  }
+  else {
+    throw new Error("qfb-boolean-mode value=" + qbm.value);
+  }
+  wait_for_all_messages_to_load(mc);
+}
+
+/**
  * Verify that tag buttons exist for exactly the given set of tag keys in the
  *  provided variable argument list.  Ordering is significant.
  */
 function assert_tag_constraints_visible() {
   // the stupid bar should be visible if any arguments are specified
   if (arguments.length && mc.e("quick-filter-bar-tab-bar").collapsed)
     throw new Error("The tag bar should not be collapsed!");
 
   let kids = mc.e("quick-filter-bar-tab-bar").childNodes;
+  let tagLength = kids.length - 1; // -1 for the qfb-boolean-mode widget
   // this is bad error reporting in here for now.
-  if (kids.length != arguments.length)
+  if (tagLength != arguments.length)
     throw new Error("Mismatch in expected tag count and actual. " +
                     "Expected " + arguments.length +
-                    " actual " + kids.length);
+                    " actual " + tagLength);
   for (let iArg = 0; iArg < arguments.length; iArg++) {
     let nodeId = "qfb-tag-" + arguments[iArg];
-    if (nodeId != kids[iArg].id)
+    if (nodeId != kids[iArg+1].id)
       throw new Error("Mismatch at tag " + iArg + " expected " + nodeId +
-                      " but got " + kids[iArg].id);
+                      " but got " + kids[iArg+1].id);
   }
 }
 
 /**
  * Verify that only the buttons corresponding to the provided tag keys are
  * checked.
  */
 function assert_tag_constraints_checked() {