fix for #40480 (problems with offscreen rows in search and filters)

fix for #78429 (status / priority pickers coming up blank) and code cleanup. some of it r=hwaara, all of it r,sr=bienvenu
2001-07-13 21:16:31 +00:00 · 2001-07-13 21:16:31 +00:00 · b3bf176eaf
--- a/mailnews/base/resources/content/mailWidgets.xml
+++ b/mailnews/base/resources/content/mailWidgets.xml
@ -221,6 +221,11 @@
          ]]>
        </getter>
      </property>
+      <constructor>
+      <![CDATA[
+        initializeTermFromId(this.id);
+      ]]>
+      </constructor>
    </implementation>
  </binding>

@ -266,7 +271,6 @@
       status
  -->
  <binding id="searchvalue" name="searchValue">
-    <!-- yeah yeah, this stuff needs to be localized. I'm working on it! -->
    <content>
      <xul:textbox flex="1" class="search-value-textbox"/>
      <xul:menulist flex="1" class="search-value-menulist">
@ -281,9 +285,9 @@
      <xul:menulist flex="1" class="search-value-menulist">
        <xul:menupopup class="search-value-popup">
          <xul:menuitem value="2" stringTag="replied" class="search-value-menuitem"/>
-          <xul:menuitem value ="1" stringTag="read" class="search-value-menuitem"/>
-          <xul:menuitem value ="1048576" stringTag="new" class="search-value-menuitem"/>
-          <xul:menuitem value ="65536" stringTag="forwarded" class="search-value-menuitem"/>
+          <xul:menuitem value="1" stringTag="read" class="search-value-menuitem"/>
+          <xul:menuitem value="1048576" stringTag="new" class="search-value-menuitem"/>
+          <xul:menuitem value="65536" stringTag="forwarded" class="search-value-menuitem"/>
        </xul:menupopup>
      </xul:menulist>
      <xul:textbox flex="1" class="search-value-textbox"/>
@ -325,9 +329,9 @@
          this.searchAttribute = attrib;
          if (attrib == nsMsgSearchAttrib.Priority) {
            var matchingPriority =
-                children[1].getElementsByAttribute("value", val.priority);
+              children[1].getElementsByAttribute("value", val.priority);
            if (matchingPriority.length > 0)
-                children[1].selectedItem = matchingPriority[0];
+              children[1].selectedItem = matchingPriority[0];
          }
          else if (attrib == nsMsgSearchAttrib.MsgStatus) {
            var matchingStatus =
@ -395,12 +399,30 @@
          ]]>
        </body>
      </method>
-	  <constructor>
+      <method name="initialize">
+        <parameter name="menulist"/>
+        <parameter name="bundle"/>
+        <body>
+          <![CDATA[
+            this.fillStringsForChildren(menulist.firstChild, bundle);
+
+            // to work around bug #78429, set and reset the selectedItem
+            var item = menulist.selectedItem;
+            menulist.selectedItem = null;
+            menulist.selectedItem = item;
+          ]]>
+        </body>
+      </method>
+      <constructor>
      <![CDATA[
        // initialize strings
        var bundle = srGetStrBundle("chrome://messenger/locale/messenger.properties");
-        this.fillStringsForChildren(document.getAnonymousNodes(this)[1].firstChild, bundle);
-        this.fillStringsForChildren(document.getAnonymousNodes(this)[2].firstChild, bundle);
+
+        // intialize the priority picker
+        this.initialize(document.getAnonymousNodes(this)[1], bundle);
+
+        // initialize the status picker
+        this.initialize(document.getAnonymousNodes(this)[2], bundle);

        // preflight the date picker to today's date
        var datePicker = document.getAnonymousNodes(this)[3];
--- a/mailnews/base/search/resources/content/searchTermOverlay.js
+++ b/mailnews/base/search/resources/content/searchTermOverlay.js
@ -17,8 +17,11 @@
 * Copyright (C) 1998 Netscape Communications Corporation. All
 * Rights Reserved.
 *
- * Contributor(s):
+ * Original Author:
 *   Alec Flett <alecf@netscape.com>
+ *
+ * Contributor(s):
+ *   Seth Spitzer <sspitzer@netscape.com>
 */

 var gTotalSearchTerms=0;
@ -28,6 +31,7 @@ var gSearchRemovedTerms = new Array;
 var gSearchScope;
 var gSearchLessButton;
 var gSearchBooleanRadiogroup;
+var gSearchTermTree;

 //
 function searchTermContainer() {}
@ -128,9 +132,9 @@ searchTermContainer.prototype = {
        searchTerm.attrib = this.searchattribute.value;
        searchTerm.op = this.searchoperator.value;
        if (this.searchvalue.value)
-          this.searchvalue.save();
+            this.searchvalue.save();
        else
-          this.searchvalue.saveTo(searchTerm.value);
+            this.searchvalue.saveTo(searchTerm.value);
        searchTerm.value = this.searchvalue.value;
        searchTerm.booleanAnd = this.booleanAnd;
    },
@ -146,24 +150,25 @@ var nsIMsgSearchTerm = Components.interfaces.nsIMsgSearchTerm;
 function initializeSearchWidgets() {
    gSearchBooleanRadiogroup = document.getElementById("booleanAndGroup");
    gSearchRowContainer = document.getElementById("searchTermList");
+    gSearchTermTree = document.getElementById("searchTermTree");
    gSearchLessButton = document.getElementById("less");
    if (!gSearchLessButton)
        dump("I couldn't find less button!");
 }

-function initializeBooleanWidgets() {
-
+function initializeBooleanWidgets() 
+{
    var booleanAnd = true;
    // get the boolean value from the first term
-    var firstTerm = gSearchTerms[0];
+    var firstTerm = gSearchTerms[0].obj;
    if (firstTerm)
        booleanAnd = firstTerm.booleanAnd;

    // target radio items have value="and" or value="or"
-    targetValue = "or";
+    var targetValue = "or";
    if (booleanAnd) targetValue = "and";

-    targetElement = gSearchBooleanRadiogroup.getElementsByAttribute("value", targetValue)[0];
+    var targetElement = gSearchBooleanRadiogroup.getElementsByAttribute("value", targetValue)[0];

    gSearchBooleanRadiogroup.selectedItem = targetElement;
 }
@ -178,11 +183,19 @@ function initializeSearchRows(scope, searchTerms)
    initializeBooleanWidgets();
 }

+function scrollToLastSearchTerm(index)
+{
+    if (index > 0)
+      gSearchTermTree.ensureIndexIsVisible(index-1);
+}
+ 
 function onMore(event)
 {
    if(gTotalSearchTerms==1)
-  gSearchLessButton .removeAttribute("disabled", "false");
+      gSearchLessButton .removeAttribute("disabled", "false");
    createSearchRow(gTotalSearchTerms++, gSearchScope, null);
+    // the user just added a term, so scroll to it
+    scrollToLastSearchTerm(gTotalSearchTerms);
 }

 function onLess(event)
@ -191,13 +204,17 @@ function onLess(event)
        removeSearchRow(--gTotalSearchTerms);
    if (gTotalSearchTerms==1)
        gSearchLessButton .setAttribute("disabled", "true");
+
+    // the user removed a term, so scroll to the bottom so they are aware of it
+    scrollToLastSearchTerm(gTotalSearchTerms);
 }

-// set scope on all visible searhattribute tags
+// set scope on all visible searchattribute tags
 function setSearchScope(scope) {
    gSearchScope = scope;
    for (var i=0; i<gSearchTerms.length; i++) {
-        gSearchTerms[i].searchattribute.searchScope = scope;
+        gSearchTerms[i].obj.searchattribute.searchScope = scope;
+        gSearchTerms[i].scope = scope;
    }
 }

@ -208,7 +225,7 @@ function booleanChanged(event) {
    var newBoolValue =
        (event.target.getAttribute("value") == "and") ? true : false;
    for (var i=0; i<gSearchTerms.length; i++) {
-        var searchTerm = gSearchTerms[i];
+        var searchTerm = gSearchTerms[i].obj;
        searchTerm.booleanAnd = newBoolValue;
    }
 }
@ -232,13 +249,11 @@ function createSearchRow(index, scope, searchTerm)
                            null, searchVal,
                            null);
    var searchrow = constructRow(rowdata);
-
    searchrow.id = "searchRow" + index;

    var searchTermObj = new searchTermContainer;
-    // is this necessary?
-    //searchTermElement.id = "searchTerm" + index;
-    gSearchTerms[gSearchTerms.length] = searchTermObj;
+
+    gSearchTerms[gSearchTerms.length] = {obj:searchTermObj, scope:scope, searchTerm:searchTerm, initialized:false};

    searchTermObj.searchattribute = searchAttr;
    searchTermObj.searchoperator = searchOp;
@ -262,20 +277,33 @@ function createSearchRow(index, scope, searchTerm)
    searchTermObj.booleanNodes = stringNodes;

    gSearchRowContainer.appendChild(searchrow);
+}

-    searchTermObj.searchScope = scope;
+function initializeTermFromId(id)
+{
+    // id is of the form searchAttr<n>
+    // strlen("searchAttr") == 10
+    // turn searchAttr<n> -> <n>
+    var index = eval(id.slice(10)); 
+    initializeTermFromIndex(index)
+}
+
+function initializeTermFromIndex(index)
+{
+    var searchTermObj = gSearchTerms[index].obj;
+
+    searchTermObj.searchScope = gSearchTerms[index].scope;
    // the search term will initialize the searchTerm element, including
    // .booleanAnd
-    if (searchTerm) {
-        searchTermObj.searchTerm = searchTerm;
-    }
-
+    if (gSearchTerms[index].searchTerm)
+        searchTermObj.searchTerm = gSearchTerms[index].searchTerm;
    // here, we don't have a searchTerm, so it's probably a new element -
    // we'll initialize the .booleanAnd from the existing setting in
    // the UI
    else
        searchTermObj.booleanAnd = getBooleanAnd();

+    gSearchTerms[index].initialized = true;
 }

 // creates a <treerow> using the array treeCellChildren as
@ -301,11 +329,16 @@ function constructRow(treeCellChildren)

 function removeSearchRow(index)
 {
-    var searchTermObj = gSearchTerms[index];
+    var searchTermObj = gSearchTerms[index].obj;
    if (!searchTermObj) {
        return;
    }

+    // if it is an existing (but offscreen) term,
+    // make sure it is initialized before we remove it.
+    if (!gSearchTerms[index].searchTerm && !gSearchTerms[index].initialized)
+        initializeTermFromIndex(index);
+
    // need to remove row from tree, so walk upwards from the
    // searchattribute to find the first <treeitem>
    var treeItemRow = searchTermObj.searchattribute;
@ -351,13 +384,21 @@ function saveSearchTerms(searchTerms, termOwner)
    var i;
    for (i = 0; i<gSearchTerms.length; i++) {
        try {
-            var searchTerm = gSearchTerms[i].searchTerm;
+            var searchTerm = gSearchTerms[i].obj.searchTerm;
+
+            // the term might be an offscreen one we haven't initialized yet
+            // if so, don't bother saving it.
+            if (!searchTerm && !gSearchTerms[i].initialized) {
+                // is an existing term, but not initialize, so skip saving
+                continue;
+            }
+
            if (searchTerm)
-                gSearchTerms[i].save();
+                gSearchTerms[i].obj.save();
            else {
                // need to create a new searchTerm, and somehow save it to that
                searchTerm = termOwner.createTerm();
-                gSearchTerms[i].saveTo(searchTerm);
+                gSearchTerms[i].obj.saveTo(searchTerm);
                termOwner.appendTerm(searchTerm);
            }
        } catch (ex) {
--- a/mailnews/base/search/src/nsMsgSearchTerm.cpp
+++ b/mailnews/base/search/src/nsMsgSearchTerm.cpp
@ -1396,6 +1396,9 @@ nsresult nsMsgResultElement::AddValue (nsMsgSearchValue *value)

 nsresult nsMsgResultElement::AssignValues (nsIMsgSearchValue *src, nsMsgSearchValue *dst)
 {
+    NS_ENSURE_ARG_POINTER(src);
+    NS_ENSURE_ARG_POINTER(dst);
+
 	// Yes, this could be an operator overload, but nsMsgSearchValue is totally public, so I'd
 	// have to define a derived class with nothing by operator=, and that seems like a bit much
 	nsresult err = NS_OK;