Closed Bug 575732 Opened 14 years ago Closed 13 years ago

mark multiple messages as read/unread needs improving (Mark -> As Read)

Categories

(Thunderbird :: Mail Window Front End, enhancement)

x86
Windows XP
enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.3a2

People

(Reporter: alon_gingold, Assigned: squib)

References

Details

Attachments

(4 files, 3 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.6) Gecko/20100625 Firefox/3.6.6 ( .NET CLR 3.5.30729; .NET4.0E)
Build Identifier: v3.05

When selecting multiple messages, the "As Read" is either marked or not marked based on the first selected message. This results in the need to sometimes repeat the action twice for the desired effect. A split into two menu options would resolve the issue.

Reproducible: Sometimes

Steps to Reproduce:
1. in a list of unread mail, select the first, and wait for it to become "read"
2. shift-click on the last unread mail to select them all.
3. mark -> as read
Actual Results:  
Since the "As read" will be checked, as the first select item has been changed to "read", the result will be a change of the first message into "un-read" state, 

Expected Results:  
The desired effect would be in the above example to mark them all as read.
This has been reported a couple times in one form or another, but I'm going to use this bug to do stuff. Attached is a patch that does what comment 0 suggests.
Attached image Screenshot
Status: UNCONFIRMED → NEW
Ever confirmed: true
clarkbw, do you think you could take a look at the UI for this? Thanks!
I confirm. Proposed solution is the best. But because changing read/unread is easy (button "M") none want fix this problem :(
Comment on attachment 455538 [details]
Screenshot

Actually, I just forgot about this bug. It looks like I also forgot to flag the screenshot for review. :clarkbw, what do you think of this?
Attachment #455538 - Flags: ui-review?(clarkbw)
Assigning this to myself, otherwise I'm bound to forget about this bug in a week or so...
Assignee: nobody → squibblyflabbetydoo
Status: NEW → ASSIGNED
Comment on attachment 455538 [details]
Screenshot

Looks good!  Sorry I missed the first message from a while back.

And if I'm reading the code right, the read / unread is available even if a single message is unread / read respectively.  i.e. a list of mixed results would offer both options.
Attachment #455538 - Flags: ui-review?(clarkbw) → ui-review+
(In reply to comment #7)
> And if I'm reading the code right, the read / unread is available even if a
> single message is unread / read respectively.  i.e. a list of mixed results
> would offer both options.

That's correct.
The options:

set as read
set as unread

Should always be shown since if I have an unread message that I've right clicked on, gets set to be read by thunderbird automatically, then having both options there so that I can then set it back to be unread if I wanted to.  If you only show one based on the current status of the message when you right clicked on it, then if I meant to set it back to unread knowing it would be set to read, I would just be able to set it back to unread without having to right click on it again to get that option.
(In reply to comment #9)
> ... if I have an unread message that I've right clicked on, gets set to be 
> read by thunderbird automatically, ...

This isn't how Thunderbird behaves. Right-clicking on a message doesn't change its read status.
(In reply to comment #10)
> (In reply to comment #9)
> > ... if I have an unread message that I've right clicked on, gets set to be 
> > read by thunderbird automatically, ...
> 
> This isn't how Thunderbird behaves. Right-clicking on a message doesn't change
> its read status.

Tools -> Options -> Advanced -> Reading & Display:

Automatically mark messages as read
    After displaying for 5 seconds
I assume you mean that you're reading a message and then right click on it to change the state and then you wait until it becomes "read" and you flip it back.

I don't think the "Mark" submenu was ever *meant* to work with status changes that happen while it's displayed; it's only by coincidence that this does what you say. For instance, it doesn't update the checked state on "As Read" when the message changes.

:clarkbw, what do you think? This seems like a case where a broken UI just happens to have accidentally allowed a certain action. If we're going to support this, and I'm not actually sure we should, then we should make sure it's done intentionally so that it works right.
That's right.  That's why instead of using tricky logic to check what status the message is in, whether it is read or unread, both options are there.  It's easier to understand and much easier to program in and it's what every other email program does.
(In reply to comment #12)
> :clarkbw, what do you think? This seems like a case where a broken UI just
> happens to have accidentally allowed a certain action. If we're going to
> support this, and I'm not actually sure we should, then we should make sure
> it's done intentionally so that it works right.

If I understand what you're saying the case we're talking about can only happen if you have the Mark > menu sitting open while the message changes because menus don't update dynamically once they are shown.  However if you moved back to the main context menu and then back to the mark menu it would have updated correctly.

What we have right now (with the logic) provides some feedback to the person about what messages are selected and what is possible since we disable the unavailable option, similarly to what other menu items do.  On the other hand it does provide this kind of use case where you could find your menu item temporarily unavailable when it should be.   Both ideas make sense but I think we should keep it how you have it working now as it's more consistent with the other menu items and how they work.  If this one menu item differs from the rest we'll later get bug reports that it doesn't work correctly and TB doesn't know what is read or unread.  I think the inconsistency here will cause more harm than good.
As the original user who opened this bug-report, I'ld like to comment that I think Carl is right. But the fact is that I use Automatically mark messages as read after 1 second (as soon as I select it). so, what happens is that I multi select some messages, as soon as I click the last message it becomes marked as "read", so I ALWAYS have ONLY the mark as "unread" available, so I ALWAYS have to select the option twice.. once to make all unread, and then again to make them read (which is what I wanted to do in the first place).
This "smart" menu context simply does not play well with such a setup, which is why other email applications simply give you two options.
:clarkbw, one option would be to listen for changes to the message and update the menu, though that might be even more confusing. Alternately, always leave "mark unread" enabled if you have the "mark read after N seconds" turned on.

Alon, if when you right-click, there are messages in both "read" and "unread" states, you'll have both options available. The only time there'd be a problem is if all selected messages change to "read" after you right-click and you're waiting to mark them unread again. I think there are better ways to perform that under this patch though, e.g. by pressing Shift+M.

Finally, I don't think it's universal that other mail apps have "mark read" and "mark unread" always available. Evolution certainly doesn't. Based on screenshots, Mail.app doesn't either. Likewise with Zimbra's webmail app. Not sure about Outlook, since screenshots seem to indicate both behavior (maybe they only added it in the newest version?).
Shift-M doesn't do anything on my Thunderbird, and I only have Mark as read with a check mark before it no matter if there are messages selected in both read and unread states (my Thunderbird is 3.1.7). Are these new changes not yet released/developed? 
This indeed sounds like an excellent solution.

I can suggest, if it really bothers anyone, that while the pop-up menu is visible, the message will NOT automatically change to read until the menu is closed, so in such a case the needed options will not change after the menu is shown. In my case (I set the auto set a read to 1 sec) I never get to right click before it changes to read, but it may be more important if you set it to 3 or more seconds..
Once again this is being made much more complicated then it needs to be.

Just include both options and let me decide which one to use.  No checking the status of the message.  No disabling the auto marking of the message as read when you've right clicked on it.  Just include both options.  Simple.

Why make something complicated that has such a simple answer?

Just because you can make it complicated doesn't mean you should.
(In reply to comment #17)
> Shift-M doesn't do anything on my Thunderbird, and I only have Mark as read
> with a check mark before it no matter if there are messages selected in both
> read and unread states (my Thunderbird is 3.1.7). Are these new changes not yet
> released/developed? 
> This indeed sounds like an excellent solution.

Shift+M is the new shortcut added by the patch attached to this bug. It's not in Thunderbird yet.

(In reply to comment #18)
> Once again this is being made much more complicated then it needs to be.
> 
> Just include both options and let me decide which one to use.

That's pretty much what one of my suggestions was: "always leave
'mark unread' enabled if you have the 'mark read after N seconds' turned on". This isn't complicated at all; it's probably 2 more lines of code total, and it would make things nice for people on either side of the fence.
2 my comments (my english is still poor):

1. Use "M"  not Shift-M, J wrote "M"  (look at shortcuts http://support.mozillamessaging.com/pl/kb/Keyboard+shortcuts  )

2. You can change "mark to read after.."

But still You have right, that current solution is bad. NEED Improvement.
(In reply to comment #16)
> :clarkbw, one option would be to listen for changes to the message and update
> the menu, though that might be even more confusing. Alternately, always leave
> "mark unread" enabled if you have the "mark read after N seconds" turned on.

Those are good solutions but it does seems to be getting a bit too complex, some users will experience the menus one way and others another; this will confuse some people and support.  Most every other email program behaves exactly as you have now, only the options which are usable are available.

Alon and Carl, I understand what you're asking for however what Jim already has is very consistent with most other Thunderbird menu items as well as most other email clients.  It's simple but simplicity also has to take into account consistency; a simple item in a more complex set of items isn't as simple as it seems by itself.

If a person marks a (already) read message as read they will likely be confused why that option was available and what effect it actually had.  This confusion is exacerbated by the fact that most other email clients don't behave this way and normally Thunderbird doesn't allow you to do something like this that has no effect.

I'm not arguing against the worthiness of the idea but that it's inconsistency will cause issues down the road and for what I believe is a marginal gain.

So Alon and Carl, a path forward on this for the both of you is to ask Jim to ensure that his patch allows for people to easily override the logic such that it always returns both commands working and that Thunderbird won't experience issues when you mark a read message as read.  Then it will be very simple to create an add-on to Thunderbird which makes it behave exactly as you are asking.
An add-on to make thunderbird behave the way all other email clients behave?  I already installed an addon called mark all read, since thunderbird doesn't provide this behavior.  It's not as good as what I want done here, but its better then how thunderbird gets it done now.  This is how useless the current implementation is to me.

I don't think users will be confused by clicking the mark as read when the message is already marked as read.  It means it should not be set to unread.  They won't be confused on why the option was there.  It's there because mark as unread is there too.  It's there so that if I highlight a bunch of messages with various statuses of being read or unread, then I can then set them all to either one and they all will conform to it.  That's what I expect.  If you make me have to right click a message, twice to set its status to read or unread, it's a bad design.

But if it gets implemented this way where once again it is useless to me, it wouldn't surprise me.  Someone other then a programmer should make this decision I believe.
(In reply to comment #21)
> (In reply to comment #16)
> > :clarkbw, one option would be to listen for changes to the message and update
> > the menu, though that might be even more confusing. Alternately, always leave
> > "mark unread" enabled if you have the "mark read after N seconds" turned on.
> 
> Those are good solutions but it does seems to be getting a bit too complex,
> some users will experience the menus one way and others another; this will
> confuse some people and support.

In terms of code complexity, the second option is really easy, though I agree that there's a potential support issue with this.

> So Alon and Carl, a path forward on this for the both of you is to ask Jim to
> ensure that his patch allows for people to easily override the logic such that
> it always returns both commands working and that Thunderbird won't experience
> issues when you mark a read message as read.  Then it will be very simple to
> create an add-on to Thunderbird which makes it behave exactly as you are
> asking.

It would probably be simpler to add a hidden pref to do this than to make the code easily extendable via an add-on (not that either option is difficult). I'm generally not one to advocate hidden prefs, but in this case it's less work for everyone involved.

I suppose making the code extendable could have broader applications, but I really can't think of any add-on except what's being discussed here, so I doubt there's much real value to that.
I rather think that showing both options either when you select both read and unread messages (or always) is the best solution, and can't figure out why that could be a problem to anyone. however, any solution which will let me do what I want (i.e. not have to select the "as read" twice to get all messages to be marked as read) is fine by me.
(In reply to comment #24)
> I rather think that showing both options either when you select both read and
> unread messages (or always) is the best solution, and can't figure out why that
> could be a problem to anyone.

This is what the above patch does (not the "always" case). If you've selected a read message, it shows "mark as unread", and if you've selected an unread message, it shows "mark as read", so if you have some of both, it shows both.

The only problem is when you select a single message and it gets marked read *after* you bring up the context menu. I think we want to fix this problem, but I disagree with clarkbw that we should let it get fixed in an add-on, simply because it's easier to fix with a hidden pref than it is to make the code extendable and then write an add-on.

Having "always show 'mark unread'" toggleable via a hidden pref also minimizes potential support issues, since it's not the default and would only show up for people who (presumably) know what they're doing (much the same as making it an add-on).
Once again I prefer to show both just because of the scenario where the message marks itself as read after you had right clicked and you then want to mark it back as unread.  Forcing both to only show up by way of a hidden preference might as well make it not exist for normal users.  And they will complain, as will I, if it is implemented the way the current patch has been designed.

So please just show both options always.  I would also add to the toolbar a mark as read button and mark as unread button.  Then I wouldn't even need to right click.
(In reply to comment #25)
>> I rather think that showing both options either when you select 
>> both read and unread messages (or always) is the best solution, and
>> can't figure out why that could be a problem to anyone.

Comment #21 seems to me to point out in some detail why that option would be a problem for people.

> The only problem is when you select a single message and it gets 
> marked read *after* you bring up the context menu. I think we want to
> fix this problem, but I disagree with clarkbw that we should let it
> get fixed in an add-on, simply because it's easier to fix with a
> hidden pref than it is to make the code extendable and then write an
> add-on.

One of our jobs as developers (and even more so, as UX designers) is to make choices for people.  Thunderbird is in an odd position, in that we allow other people to make different choices through addons, but that doesn't mean that we don't have to make the decision of how we want the base product to behave.

> Having "always show 'mark unread'" toggleable via a hidden pref also 
> minimizes potential support issues, since it's not the default and
> would only show up for people who (presumably) know what they're
> doing (much the same as making it an add-on).

It would, however, give us more code to support, instead of spreading that burden onto people who are more interested in maintaining that behaviour.  And it's not really an either-or situation here.  Making the code extensible could let users come up with all sorts of different things to do.  (Like "Show both buttons unless it's a Tuesday, and the second message is green."  Sure, that's a silly example, but I'm often surprised by what people want to do.  ;)

Finally, for the record, I agree with Bryan; I would much rather see the option to always show both be implemented as an addon.

Thanks,
Blake.
I thought about it a bit and I managed to come up with another extension: disable mark read/unread for "locked" messages, so that's enough for me to be convinced. I'll just make it easy to hook into. I suppose I ought to think about how to make all the other mark functions (e.g. "mark thread as read") hookable too, but that's probably not essential.

Carl, not all is lost, since with the patch applied, you can press Shift+M to mark messages as unread no matter what their state is (well, technically Shift+M will just do nothing on unread messages). I suppose it's not perfect since you need to use the keyboard instead of the mouse, but I think it will be sufficient for most people.

If you really want to use the mouse, there's already a "Toggle read" toolbar button you can add to the main toolbar, and this will still be around after the patch. Basically, instead of right-clicking and waiting for the status to change to "read" and then clicking "mark unread", you can just wait for the status to change and then click the toolbar button.

Hopefully this more-or-less satisfies everyone because I'd rather stop thinking about how to handle this bug now. :)
I don't want to have to remember keyboard shortcuts to set a message as read or unread.  I want it done the way every other email client does it.  Which is to show both options.  I shouldn't have to install an addon to get such a basic feature working.

This wouldn't be an issue if there was a single person making the decision instead of letting everyone have their input no matter if they're good at user interface design or not.
Carl, both Apple's Mail.app and Google's GMail web interface only list "Mark as Read" or "Mark as Unread" and not both at the same time, so "every other email client" seem to do it the way Thunderbird does.  Which email clients were you referring to, and would you mind posting screenshots of them for me?  (That's an honest request, I'm interested in seeing what sort of UIs other people are implementing.)

(On a more personal note, in my opinion "Everyone else does it this way" isn't a great argument to make.  There's something to be said for consistency, I agree, but there's also something to be said for being different if it makes the user's life easier.  Like if you can remove a menu option that makes no sense, for example.  ;)

Thanks,
Blake.
(In reply to comment #29)
> I don't want to have to remember keyboard shortcuts to set a message as read or
> unread.

You don't have to remember keyboard shortcuts, since as I mentioned in the third paragraph of comment 28, you can do what you need by clicking a toolbar button (you have to add this button to the main toolbar, of course).

> This wouldn't be an issue if there was a single person making the decision
> instead of letting everyone have their input no matter if they're good at user
> interface design or not.

There is a single person making the decision: clarkbw, the user experience lead for Thunderbird. He's been involved in the process for pretty much the whole time, and it's his decision that we ultimately went with. While you may personally disagree with that decision, this isn't an example of design by the masses.

(In reply to comment #30)
> Carl, both Apple's Mail.app and Google's GMail web interface only list "Mark as
> Read" or "Mark as Unread" and not both at the same time, so "every other email
> client" seem to do it the way Thunderbird does.

See also comment 16. Evolution works the same way as proposed here, and it looks like Zimbra and older versions of Outlook do too. I'm not sure about Outlook 2010, though.

More generally, while I understand the reasons for wanting "mark as read" and "mark as unread" always visible, I think that people who need this behavior are adequately served by the proposed patch, whether they want to use the keyboard or the mouse (again, see comment 28).
Here is the email client eudora showing read and unread on a single message allowing me to select either one no matter what.
Attached patch Add extension points (obsolete) — Splinter Review
This patch makes the commands easier to extend and fixes a couple errors in the strings. I'm not sure what kind of tests are necessary though, or if we even need tests for this (I suppose they couldn't hurt).
Attachment #455535 - Attachment is obsolete: true
Attachment #500912 - Flags: review?(bwinton)
Comment on attachment 500912 [details] [diff] [review]
Add extension points

So, this patch really looks to me like you've added some new UI elements and behaviours, so I'm going to request a UI-review from Bryan.

And we definitely need tests for this.  I suggest some Mozmill tests that use the various menu items and buttons to mark messages read and unread, and checks that the correct things are enabled and disabled.

>+function MsgMarkMsgAsRead(read)
>+{
>+  if (read == undefined)
>+      read = !gFolderDisplay.selectedMessage.isRead;

That should be a 2-space indent.

Finally, I notice that the accesskeys of "M" and "Shift-M" for the "Message » Mark" menu don't seem to work on Linux.

And so, given that bug and the lack of tests, I think I'm going to have to r- the patch.  (But I am mostly happy with it, and would still like Bryan to do the ui-r.)

Thanks,
Blake.
Attachment #500912 - Flags: ui-review?(clarkbw)
Attachment #500912 - Flags: review?(bwinton)
Attachment #500912 - Flags: review-
(In reply to comment #34)
> Finally, I notice that the accesskeys of "M" and "Shift-M" for the "Message »
> Mark" menu don't seem to work on Linux.

Is there something special you're doing here? Hitting M or Shift-M works fine for me.
Attached patch Now with tests! (obsolete) — Splinter Review
Here are some tests. I'm not sure if these are quite enough, but it checks all the basic cases with the hotkeys. I didn't attempt to test activating this stuff by any other means, nor did I test whether the menu items are properly grayed-out. If more tests are required, let me know.
Attachment #500912 - Attachment is obsolete: true
Attachment #500912 - Flags: ui-review?(clarkbw)
Attachment #502574 - Flags: ui-review?(clarkbw)
Attachment #502574 - Flags: review?(bwinton)
As mentioned in IRC, my problem was entirely on my end.  I'll get to this review after Bryan does the ui-review.

Thanks,
Blake.
(In reply to comment #34)
> Comment on attachment 500912 [details] [diff] [review]
> Add extension points
> 
> So, this patch really looks to me like you've added some new UI elements and
> behaviours, so I'm going to request a UI-review from Bryan.

What are these changes?
(In reply to comment #38)
> (In reply to comment #34)
> > Comment on attachment 500912 [details] [diff] [review]
> > Add extension points
> > 
> > So, this patch really looks to me like you've added some new UI elements and
> > behaviours, so I'm going to request a UI-review from Bryan.
> 
> What are these changes?

Actually, it looks like you ui-reviewed the only real change above, but here's exactly what's happened, just in case:

* Message -> Mark now shows "As Read" and "As Unread"
* The Mark toolbar button's menu now show "As Read" and "As Unread"
* Clicking on the Mark toolbar button still toggles the read status just like before
Comment on attachment 502574 [details] [diff] [review]
Now with tests!

ok, looks good then
Attachment #502574 - Flags: ui-review?(clarkbw) → ui-review+
Comment on attachment 502574 [details] [diff] [review]
Now with tests!

(In reply to comment #36)
> Created attachment 502574 [details] [diff] [review]
> Now with tests!
> 
> Here are some tests. I'm not sure if these are quite enough, but it checks all
> the basic cases with the hotkeys. I didn't attempt to test activating this
> stuff by any other means, nor did I test whether the menu items are properly
> grayed-out. If more tests are required, let me know.

Well, I was hoping I could catch you out for using cmdKey or altKey instead of accelKey, but you managed to avoid my trap.  ;)

I would like to see something about the menu items enabling/disabling, but I won’t block the patch from landing for those.

>+++ b/mail/test/mozmill/folder-display/test-message-commands.js
>@@ -42,32 +42,130 @@
>+function test_mark_n_read() {
>+  be_in_folder(commandFolder);

I think "unreadFolder" might be a better name.

>+function test_mark_n_read_mixed() {
>+  be_in_folder(commandFolder);
>+  select_click_row(0);
>+  let curMessages = select_shift_click_row(1);
>+  

Trailing spaces!  :)

The things I’ve mentioned are all really minor, so I’m going to say r=me and thank you for your work!

Later,
Blake.
Attachment #502574 - Flags: review?(bwinton) → review+
This fixes the nits and tests whether the menu items are enabled/disabled properly.
Attachment #502574 - Attachment is obsolete: true
Attachment #502826 - Flags: ui-review+
Attachment #502826 - Flags: review?(bwinton)
Comment on attachment 502826 [details] [diff] [review]
Test menuitem states too

You had my r+ already, so you didn't really need to re-request it, but I've checked over the additions, and they seem good.

Thanks,
Blake.
Attachment #502826 - Flags: review?(bwinton) → review+
Ah, alright. Just making sure. Looks like this is done now.
Keywords: checkin-needed
:standard8, this should fix the test failures on Mac, but probably not the ones from the Tryserver, which may just be randomness. Main differences: this checks the context menu now and uses click_menus_in_sequence instead of trying to play games with keypresses.
Attachment #504010 - Flags: review?(bugzilla)
Attachment #504010 - Flags: review?(bugzilla) → review+
key m doesn’t work any longer to mark mail as unread.

Maybe it isn’t possible to use a key for two Cmds?

398 <!ENTITY markAsReadCmd.key "m">

401 <!ENTITY markAsUnreadCmd.key "m">

http://mxr.mozilla.org/comm-central/source/mail/locales/en-US/chrome/messenger/messenger.dtd#396
(In reply to comment #46)
> key m doesn’t work any longer to mark mail as unread.

It's Shift+M (note the end of this line):

+  <key id="key_markAsUnread" key="&markAsUnreadCmd.key;"             oncommand="goDoCommand('cmd_markAsUnread');" modifiers="shift"/>

> Maybe it isn’t possible to use a key for two Cmds?

It's the same method used for mark as junk/not junk, so I just followed the established way of representing this.
This was landed last Friday:

http://hg.mozilla.org/comm-central/rev/cc7fcc38e0f2

Unfortunately there were unit tests, some were fixed by attachment 504010 [details] [diff] [review]:

http://hg.mozilla.org/comm-central/rev/4f7331f00b19

Then they were some still failing so I backed them all out:

http://hg.mozilla.org/comm-central/rev/99a15f0f16b1

Today I re-ran the fixed tests on try-server and they passed, so they are now re-enabled on the tree:

http://hg.mozilla.org/comm-central/rev/476b91f50827

We still need to fix the tests that use the check_read_status function.
Keywords: checkin-needed
Target Milestone: --- → Thunderbird 3.3a2
(In reply to comment #48)
> We still need to fix the tests that use the check_read_status function.

Do these tests still fail only on Mac, or are they failing on all platforms?
(In reply to comment #49)
> (In reply to comment #48)
> > We still need to fix the tests that use the check_read_status function.
> 
> Do these tests still fail only on Mac, or are they failing on all platforms?

Mac and Windows.
Depends on: 627095
there's probably existing litmus tests that will need to be altered for "M", shift+M
Flags: in-litmus?
OS: Windows 7 → Windows XP
Depends on: 628768
(In reply to comment #51)
> there's probably existing litmus tests that will need to be altered for "M",
> shift+M

bug 	628768 reverts this it seems.
Flags: in-litmus?
Jim, have we fixed this now?
Should be fixed now.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Blocks: 768025
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: