233027 - multi-message selection: mark as read timer not cleared

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[kevin fisher]

User-Agent:       
Build Identifier: Mozilla Thunderbird 0.5a (20040128)

When selecting multiple messages by first clicking on one unread message,
holding shift, then clicking on any other message, all selected messages are
marked as 'read' if the multiple-message selection was made before the 'mark as
read after X seconds' timer expired for the first message you selected.

from mscott: "we are supposed to be clearing the timer as soon as you try to
select more than one element in the thread pane or change the selected message.
Sounds like we are not doing that."

Reproducible: Always
Steps to Reproduce:
1. set some time > 0 for 'mark as read after X seconds' setting
2. select an unread message in the thread pane
3. hold shift and select another message in the thread pane (before X seconds)
4. wait for remainder of X seconds

Actual Results:  
all messages are marked as 'read'

Expected Results:  
should leave read status alone for all messages

Comment 1

[kevin fisher]

should read:
-------
Actual Results:
all *selected* messages are marked as 'read'

Expected Results:
should leave read status alone for all *selected* messages

Comment 2

[Erik Arvidsson]

confirming

Comment 3

[Erik Arvidsson]

This seems to be working when using the mouse now.

Using the keyboard to select multiple messages keeps the first message shown in
the message pane and thus TB does not clear the message. The real problem seems
to be that multiple selection using the keyboard does not fire the change that
tells TB to update the message pane and therefore the first message is loaded
and the read timer is fired and marks all selected messgas as read.

I'm setting the priority to major since this is results in data loss.

Comment 4

[Erik Arvidsson]

I filed a seperate bug for the keyboard selection bug which, if fixed, should
fix this bug.

Comment 5

[Jeff Walden [:Waldo]]

*** Bug 244126 has been marked as a duplicate of this bug. ***

Comment 6

[Erik Arvidsson]

This leads to data loss due to non read messages becoming marked as read and
therefore these might be missed.

Comment 7

[blujay]

Just noticed this myself on Tb 1.0.

Comment 8

[Mike Cowperthwaite]

(In reply to comment #3)
> This seems to be working when using the mouse now.

I'm not seeing any difference in this behavior between selecting via mouse and 
selecting via keyboard.  TB 1.0, Win2K.

Comment 9

[Mike Cowperthwaite]

*** Bug 254691 has been marked as a duplicate of this bug. ***

Comment 10

[Erik Arvidsson]

I don't know which build I was using at the time but mark as read is still
horendously broken even using the mouse to select. (version 1.0 (20041206))

This should probably also be hardware All and OS All but I don't have acces to
anything but WinXP at the moment. Can someone test this on an alternative
OS/hardware?

Comment 11

[Mike Cowperthwaite]

*** Bug 286032 has been marked as a duplicate of this bug. ***

Comment 12

[Mike Cowperthwaite]

*** Bug 290388 has been marked as a duplicate of this bug. ***

Comment 13

[Mike Cowperthwaite]

*** Bug 266988 has been marked as a duplicate of this bug. ***

Comment 14

[Erik Arvidsson]

Attachment: Clear read timer in a few more places

Clear the read timer any time the message view is cleared or loaded as well as
when the user manually changes the read state.

This patch is not made on the cvs tree but on the latest nightly. If required I
can spend several hours downloading and syncing a cvs tree?

Comment 15

[Jeff Walden [:Waldo]]

(In reply to comment #14)
> This patch is not made on the cvs tree but on the latest nightly. If required
> I can spend several hours downloading and syncing a cvs tree?

There's certainly no need to download the entire tree to make a patch that
changes isolated bits of code.  I regularly use bite-sized cvs checkouts in
order to get a small tree that's clean before applying a patch and committing it
to source.  It'll be a little more hassle for you because tree paths don't match
with the paths stored in nightlies, but it's not insurmountable.

Anyway, running the following command should get you all the files you need (and
more - I don't have the time to make this a precision checkout) in order to make
the patch:

cvs -q -z3 co mozilla/mail/base

From there, in order to figure out the files you need to modify you'll need to
figure out the mapping of files in the nightly to files in source.  The file
mozilla/mail/base contains this mapping.  The first string on each filename line
is the location of the file within the JAR you unzipped to get at the source. 
The second is the location of the file relative to the current source directory.
 From there, take the diffs you've made and use them to move your changes into
the portion of trunk source code you've downloaded.  (You probably can't just
copy the changed files over the files in the tree, because the files in the tree
may be preprocessed [I haven't looked] in ways that make the files specific to
the nightly's target platform [and thus a copy would make the files
non-platform-agnostic].  That said, there's no harm in trying it, so go ahead if
you want to try saving time.)

After your changes are in your tree, running:

cvs diff -u8 mozilla/mail/base/ > msg-selection-patch.txt

...from the directory where you first ran the checkout command should work just
fine for creating a diff that you can post here that's immediately usable.

If you have further questions email me privately.  Using this partial-tree
checkout method should save you a substantial amount of time over downloading
the whole tree for one 3KB patch to 4 files.

Comment 16

[Jeff Walden [:Waldo]]

(In reply to comment #15)
> The file mozilla/mail/base contains this mapping.

Sorry for bugspam - that should be <mozilla/mail/base/jar.mn>.

Comment 17

[Erik Arvidsson]

Attachment: Same patch as before but on the CVS tree

Thanks goes to Jeff Walden for showing how easy things can be

Comment 18

[Erik Arvidsson]

Sorry for the bug spam but I'm interested in getting some reviews so that this
dataloss bug can be closed.

Comment 19

[Scott MacGregor]

Comment on attachment 188216 [details] [diff] [review]
Same patch as before but on the CVS tree

thanks for the patch Erik. I just checked this in with a couple minor changes:

1) I got rid of some extraneous braces which came out of this change. I also
finished off the method call here:

       if (markReadOnADelay && wintype == "mail:3pane") // only use the timer
if viewing using the 3-pane preview pane and the user has set the pref
+      {
+	 ClearPendingReadTimer

}

Comment 20

[:aceman]

Is there a seamonkey version of this bug?