498747 - Fix selection of threads & messages

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[David Ascher (:davida)]

Attachment: patch and test

A minor regression from bug 474701 is that if the selection starts with a thread, and includes messages not in that thread, that the multi-message selection code thinks it's still a thread.  Similarly, if a message is the first in the selection, then selecting threads displays each message in a thread instead of the thread proxy (with a grey border).

Mozmill test, and fix, attached. (w00t)

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 383553 [details] [diff] [review]
patch and test

Fantastic.

Since I am on the hook to commit it for you, I had the following nit which I fixed for you:

diff --git a/mail/test/mozmill/shared-modules/test-folder-display-helpers.js b/mail/test/mozmill/shared-modules/test-folder-display-helpers.js
--- a/mail/test/mozmill/shared-modules/test-folder-display-helpers.js
+++ b/mail/test/mozmill/shared-modules/test-folder-display-helpers.js
@@ -1008,17 +1008,17 @@ function make_display_grouped() {
  * assert that the multimessage/thread summary view contains
  * the specified number of elements of the specified class.
  *
  * @param aClassName: the class to use to select
  * @param aNumElts: the number of expected elements that have that class
  */
 
 function assert_summary_contains_N_divs(aClassName, aNumElts) {
-  let htmlframe = mc.window.document.getElementById('multimessage');
+  let htmlframe = mc.e('multimessage');
   let matches = htmlframe.contentDocument.getElementsByClassName(aClassName);
   if (matches.length != aNumElts)
     throw new Error("Expected to find " + aNumElts + " elements with class " +
                     aClassName + ", found: " + matches.length);
 }
 
 
 function throw_and_dump_view_state(aMessage, aController) {

If you do not like this change to use a convenience function we expose, let me know and I will un-do it.

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

pushed: http://hg.mozilla.org/comm-central/rev/49f947d1b97f

Comment 3

[Bryan Clark (DevTools PM) [:clarkbw]]

Attachment: declare the variable i

In windows I'm now seeing this warning:

Warning: assignment to undeclared variable i
Source File: chrome://messenger/content/messageDisplay.js
Line: 300

This patch fixes the missing i variable declaration.

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

Thanks for the fix!

pushed: http://hg.mozilla.org/comm-central/rev/d92cf1f50471