Bug 747284 - In the filter editor, show the real execution order of actions. r=rkent, ui-r=bwinton

mail/locales/en-US/chrome/messenger/FilterEditor.dtd

@@ -27,6 +27,9 @@
 <!ENTITY filterActionDesc.label "Perform these actions:">
 <!ENTITY filterActionDesc.accesskey "P">
 
+<!ENTITY filterActionOrderWarning.label "Note: Filter actions will be run in a different order.">
+<!ENTITY filterActionOrder.label "See execution order">
+
 <!-- New Style Filter Rule Actions -->
 <!ENTITY moveMessage.label "Move Message to">
 <!ENTITY copyMessage.label "Copy Message to">

mail/locales/en-US/chrome/messenger/filter.properties

@@ -29,7 +29,16 @@ searchTermsInvalidTitle=Search Terms Invalid
 # %1$S=search attribute name from the invalid rule
 # %2$S=search operator from the bad rule
 searchTermsInvalidRule=This filter cannot be saved because the search term "%1$S %2$S" is invalid in the current context.
+# LOCALIZATION NOTE(filterActionOrderExplanation)
+# Keep the \n\n that mean 2 linebreaks.
+filterActionOrderExplanation=When a message matches this filter the actions will be run in this order:\n\n
+filterActionOrderTitle=Real action order
+## LOCALIZATION NOTE(filterActionItem):
+# %1$S=sequence number of the action, %2$S=action text, %3$S=action argument
+filterActionItem=%1$S. %2$S %3$S\n
 
+## LOCALIZATION NOTE(filterCountVisibleOfTotal):
+# %1$S=number of matching filters, %2$S=total number of filters
 filterCountVisibleOfTotal=%1$S of %2$S
 ## LOCALIZATION NOTE(filterCountItems): Semi-colon list of plural forms.
 ## See: http://developer.mozilla.org/en/docs/Localization_and_Plurals

mailnews/base/search/content/FilterEditor.js

@@ -29,12 +29,17 @@ var gFilterActionStrings = ["none", "movemessage", "setpriorityto", "deletemessa
                             "fetchfrompopserver", "copymessage", "addtagtomessage",
                             "ignoresubthread", "markasunread"];
 
-var nsMsgFilterAction = Components.interfaces.nsMsgFilterAction;
+// A temporary filter with the current state of actions in the UI.
+let gTempFilter = null;
+// A nsIArray of the currently defined actions in the order they will be run.
+let gActionListOrdered = null;
 
 var gFilterEditorMsgWindow = null;
 
-const nsMsgFilterType = Components.interfaces.nsMsgFilterType;
-const nsMsgSearchScope = Components.interfaces.nsMsgSearchScope;
+const nsMsgFilterAction = Components.interfaces.nsMsgFilterAction;
+const nsMsgFilterType   = Components.interfaces.nsMsgFilterType;
+const nsIMsgRuleAction  = Components.interfaces.nsIMsgRuleAction;
+const nsMsgSearchScope  = Components.interfaces.nsMsgSearchScope;
 
 function filterEditorOnLoad()
 {
@@ -320,7 +325,7 @@ function initializeDialog(filter)
     newActionRow.className = 'ruleaction';
     gFilterActionList.appendChild(newActionRow);
     newActionRow.setAttribute('value',
-        filterAction.type == Components.interfaces.nsMsgFilterAction.Custom ?
+        filterAction.type == nsMsgFilterAction.Custom ?
         filterAction.customId : gFilterActionStrings[filterAction.type]);
   }
 
@@ -484,10 +489,79 @@ function saveFilter()
   return true;
 }
 
+/**
+ * Check if the list of actions the user created will be executed in a different order.
+ * Exposes a note to the user if that is the case.
+ */
+function checkActionsReorder()
+{
+  setTimeout(_checkActionsReorder, 0);
+}
+
+/**
+ * This should be called from setTimeout otherwise some of the elements calling
+ * may not be fully initialized yet (e.g. we get ".saveToFilter is not a function").
+ * It is OK to schedule multiple timeouts with this function.
+ */
+function _checkActionsReorder() {
+  // Create a temporary disposable filter and add current actions to it.
+  if (!gTempFilter)
+    gTempFilter = gFilterList.createFilter("");
+  else
+    gTempFilter.clearActionList();
+
+  for (let index = 0; index < gFilterActionList.itemCount; index++)
+    gFilterActionList.getItemAtIndex(index).saveToFilter(gTempFilter);
+
+  // Now get the actions out of the filter in the order they will be executed in.
+  gActionListOrdered = gTempFilter.sortedActionList;
+
+  // Compare the two lists.
+  let statusBar = document.getElementById("statusbar");
+  for (let index = 0; index < gActionListOrdered.length; index++) {
+    if (index != gTempFilter.getActionIndex(
+        gActionListOrdered.queryElementAt(index, nsIMsgRuleAction)))
+    {
+      // If the lists are not the same unhide the status bar and show warning.
+      statusBar.style.visibility = "visible";
+      return;
+    }
+  }
+
+  statusBar.style.visibility = "hidden";
+}
+
+/**
+ * Show a dialog with the ordered list of actions.
+ * The fetching of action label and argument is separated from checkActionsReorder
+ * function to make that one more lightweight. The list is built only upon
+ * user request.
+ */
+function showActionsOrder()
+{
+  // Fetch the actions and arguments as a string.
+  let actionStrings = [];
+  for (let index = 0; index < gFilterActionList.itemCount; index++)
+    gFilterActionList.getItemAtIndex(index).getActionStrings(actionStrings);
+
+  // Present a nicely formatted list of action names and arguments.
+  let actionList = gFilterBundle.getString("filterActionOrderExplanation");
+  for (let i = 0; i < gActionListOrdered.length; i++) {
+    let actionIndex = gTempFilter.getActionIndex(
+      gActionListOrdered.queryElementAt(i, nsIMsgRuleAction));
+    let action = actionStrings[actionIndex];
+    actionList += gFilterBundle.getFormattedString("filterActionItem",
+      [(i + 1), action.label, action.argument]);
+  }
+
+  Services.prompt.confirmEx(window,
+                            gFilterBundle.getString("filterActionOrderTitle"),
+                            actionList, Services.prompt.BUTTON_TITLE_OK,
+                            null, null, null, null, {value:false});
+}
 
 function AssignMeaningfulName()
 {
-
   // termRoot points to the first search object, which is the one we care about.
   let termRoot = gSearchTerms[0].obj;
   // stub is used as the base name for a filter.

mailnews/base/search/content/FilterEditor.xul

@@ -101,4 +101,12 @@
     </listbox>
   </vbox>
 
+  <vbox id="statusbar" style="visibility: hidden;">
+    <hbox align="center">
+      <label>
+        &filterActionOrderWarning.label;
+      </label>
+      <label class="text-link" onclick="showActionsOrder();">&filterActionOrder.label;</label>
+    </hbox>
+  </vbox>
 </dialog>

mailnews/base/search/content/searchWidgets.xml

@@ -11,6 +11,7 @@
       -gFilter from FilterEditor.js
       -gCustomActions from FilterEditor.js
       -gFilterType from FilterEditor.js
+      -checkActionsReorder from FilterEditor.js
       -GetMsgFolderFromUri, SetFolderPickerElement from msgFolderPickerOverlay.js
 -->
 
@@ -278,6 +279,7 @@
       <handler event="command">
         <![CDATA[
           this.parentNode.setAttribute('value', this.menulist.value);
+          checkActionsReorder();
         ]]>
       </handler>
 
@@ -426,6 +428,7 @@
                     .value = filterActionStr;
             this.mRuleActionTargetInitialized = true;
             this.clearInitialActionIndex();
+            checkActionsReorder();
           ]]>
         </body>
       </method>
@@ -524,6 +527,24 @@
         </body>
       </method>
 
+      <method name="getActionStrings">
+        <parameter name="aActionStrings"/>
+        <body>
+          <![CDATA[
+            // Collect the action names and arguments in a plain string form.
+            let actionTarget = document.getAnonymousNodes(this)[1];
+            let actionItem = document.getAnonymousNodes(actionTarget);
+
+            aActionStrings.push({
+              label: document.getAnonymousNodes(this.mRuleActionType)[0].label,
+              argument: actionItem ?
+                        (actionItem[0].label ?
+                         actionItem[0].label : actionItem[0].value) : ""
+            });
+          ]]>
+        </body>
+      </method>
+
       <method name="updateRemoveButton">
         <body>
           <![CDATA[
@@ -543,6 +564,7 @@
 
             // make sure the first remove button is enabled
             this.updateRemoveButton();
+            checkActionsReorder();
           ]]>
         </body>
       </method>
@@ -556,6 +578,7 @@
               listBox.removeChild(this);
             // can't use 'this' as it is destroyed now
             listBox.getItemAtIndex(0).updateRemoveButton();
+            checkActionsReorder();
           ]]>
         </body>
       </method>

mailnews/base/search/public/nsIMsgFilter.idl

@@ -43,7 +43,7 @@ interface nsIMsgRuleAction : nsISupports {
   
 };
 
-[scriptable, uuid(ed4b7a8e-3190-4b0b-b842-d4f1ca65216f)]
+[scriptable, uuid(ed4b7a8e-3190-4b0b-b842-d4f1ca652170)]
 interface nsIMsgFilter : nsISupports {
     attribute nsMsgFilterTypeType filterType;
     /**
@@ -96,6 +96,8 @@ interface nsIMsgFilter : nsISupports {
 
     nsIMsgRuleAction getActionAt(in unsigned long aIndex);
 
+    long getActionIndex(in nsIMsgRuleAction aAction);
+
     void appendAction(in nsIMsgRuleAction action);
 
     readonly attribute unsigned long actionCount;

mailnews/base/search/src/nsMsgFilter.cpp

@@ -389,6 +389,15 @@ nsMsgFilter::GetActionAt(uint32_t aIndex, nsIMsgRuleAction **aAction)
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsMsgFilter::GetActionIndex(nsIMsgRuleAction *aAction, int32_t *aIndex)
+{
+  NS_ENSURE_ARG_POINTER(aIndex);
+
+  *aIndex = m_actionList.IndexOf(aAction);
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 nsMsgFilter::GetActionCount(uint32_t *aCount)
 {

suite/locales/en-US/chrome/mailnews/FilterEditor.dtd

@@ -27,6 +27,9 @@
 <!ENTITY filterActionDesc.label "Perform these actions:">
 <!ENTITY filterActionDesc.accesskey "P">
 
+<!ENTITY filterActionOrderWarning.label "Note: Filter actions will be run in a different order.">
+<!ENTITY filterActionOrder.label "See execution order">
+
 <!-- New Style Filter Rule Actions -->
 <!ENTITY moveMessage.label "Move Message to">
 <!ENTITY copyMessage.label "Copy Message to">

suite/locales/en-US/chrome/mailnews/filter.properties

@@ -28,6 +28,13 @@ searchTermsInvalidTitle=Search Terms Invalid
 # %1$S=search attribute name from the invalid rule
 # %2$S=search operator from the bad rule
 searchTermsInvalidRule=This filter cannot be saved because the search term "%1$S %2$S" is invalid in the current context.
+# LOCALIZATION NOTE(filterActionOrderExplanation)
+# Keep the \n\n that mean 2 linebreaks.
+filterActionOrderExplanation=When a message matches this filter the actions will be run in this order:\n\n
+filterActionOrderTitle=Real action order
+## LOCALIZATION NOTE(filterActionItem):
+# %1$S=sequence number of the action, %2$S=action text, %3$S=action argument
+filterActionItem=%1$S. %2$S %3$S\n
 
 # for junk mail logging / mail filter logging
 # LOCALIZATION NOTE(junkLogDetectStr)