Closed
Bug 768025
Opened 12 years ago
Closed 12 years ago
Mark multiple messages as read/unread needs improving (Mark -> As Read)
Categories
(SeaMonkey :: MailNews: Message Display, enhancement)
SeaMonkey
MailNews: Message Display
Tracking
(Not tracked)
RESOLVED
FIXED
seamonkey2.19
People
(Reporter: philip.chee, Assigned: jeniferwang)
References
(Blocks 1 open bug)
Details
(Whiteboard: [good first bug][mentor=IanN][lang=js/xul][level=journeyman][2012 Fall Equinox])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
iannbugzilla
:
review+
|
Details | Diff | Splinter Review |
From Thunderbird Bug 575732:
> 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.
Also please note followup regression fixes to Bug 575732:
TB Bug 628768 - mark as read toggle broken.
TB Bug 641253 - Message > Mark menu shouldn't be disabled when no messages are selected
Hi, though I am a newbie here, still I want to contribute my part. Can u give me some clue about how to start dealing with this bug? Thanks.
Reporter | ||
Comment 2•12 years ago
|
||
First you need to be able to build seamonkey. Please read <https://developer.mozilla.org/en-US/docs/Simple_SeaMonkey_build>. Also you can join us on IRC: irc://moznet/seamonkey where it's easier to ask questions and get replies. For help in getting seamonkey to build it's also useful to ask in irc://moznet/introduction .
(In reply to Liu from comment #1)
> Hi, though I am a newbie here, still I want to contribute my part. Can u
> give me some clue about how to start dealing with this bug? Thanks.
Did you get anywhere with this?
Sorry, I was trying to fix another bug. Haven't gotten concrete process for this one.
(In reply to Ian Neal from comment #3)
> (In reply to Liu from comment #1)
> > Hi, though I am a newbie here, still I want to contribute my part. Can u
> > give me some clue about how to start dealing with this bug? Thanks.
>
> Did you get anywhere with this?
Updated•12 years ago
|
Whiteboard: [good first bug][mentor=IanN][lang=js/xul][level=journeyman] → [good first bug][mentor=IanN][lang=js/xul][level=journeyman][2012 Fall Equinox]
Reporter | ||
Comment 6•12 years ago
|
||
> zephyr 2012-10-29 13:12:48 PDT
> I wish to work upon this bug.
Hi! Thanks for your offer. Have you read Comment 2 yet?
(In reply to Philip Chee from comment #6)
> > zephyr 2012-10-29 13:12:48 PDT
> > I wish to work upon this bug.
> Hi! Thanks for your offer. Have you read Comment 2 yet?
yes,and I have built the /comm-central from mercurial.
Reporter | ||
Comment 8•12 years ago
|
||
That's great! Here is the Thunderbird changeset that we need to adapt to SeaMonkey:
http://hg.mozilla.org/comm-central/rev/cc7fcc38e0f2
Our equivalent to the TB mail3PaneWindowCommands.js etc resides here in /suite/mailnews/:
http://mxr.mozilla.org/comm-central/find?text=&string=%2Fsuite%2Fmailnews%2F
The Thunderbird patch includes some mozmill tests. Since we don't run mozmill tests you can ignore that part.
The best way to contact us is to hop on to IRC and connect to the mozilla server:
irc://moznet/seamonkey. Depending on the timezone we may or may not be around so please be patient. Most of the SeaMonkey developers are in the EU and a couple of us are in Asia.
(In reply to Philip Chee from comment #8)
> That's great! Here is the Thunderbird changeset that we need to adapt to
> SeaMonkey:
>
> http://hg.mozilla.org/comm-central/rev/cc7fcc38e0f2
>
> Our equivalent to the TB mail3PaneWindowCommands.js etc resides here in
> /suite/mailnews/:
> http://mxr.mozilla.org/comm-central/find?text=&string=%2Fsuite%2Fmailnews%2F
>
> The Thunderbird patch includes some mozmill tests. Since we don't run
> mozmill tests you can ignore that part.
>
> The best way to contact us is to hop on to IRC and connect to the mozilla
> server:
> irc://moznet/seamonkey. Depending on the timezone we may or may not be
> around so please be patient. Most of the SeaMonkey developers are in the EU
> and a couple of us are in Asia.
what is your irc nick?
Reporter | ||
Comment 10•12 years ago
|
||
zephyr: on IRC I'm Ratty (or RattyAway) even if I'm not around leave me a message and I'll see it in the channel logs.
Assignee | ||
Comment 11•12 years ago
|
||
Hi,
I would like to work on this bug with a friend. I'm currently building the environment and I'll get back with questions here or in IRC :)
Assignee | ||
Comment 12•12 years ago
|
||
I was able to build SeaMonkey and I'm trying to understand the code and figure out where I should identify the issue at. At http://mxr.mozilla.org/comm-central/source/suite/mailnews/mailWindowOverlay.js#867 , what does gDBView and hdrForFirstSelectedMessage mean and what do they do?
Assignee | ||
Comment 13•12 years ago
|
||
(Worked on this with vchinen)
Instead of checking the first message, we check all selected messages to determine if the they should mark as read or unread.
Attachment #718262 -
Flags: review?(iann_bugzilla)
Comment 14•12 years ago
|
||
Comment on attachment 718262 [details] [diff] [review]
First patch fix submission
IIUC, this patch returns false if there is only one message and it is not read, or if there are more than one and any messages other than the first one are not read. It returns true if either there is only one message and it is read, or if there are more than one and all of them other than the first (which can be anything) are read. I don't understand the logic.
I guess there is an off-by-one error in the "for" statement, where "let i = 1" should be "let i = 0"; but then the whole test on .length == 1 becomes superfluous.
Attachment #718262 -
Flags: feedback-
Assignee | ||
Comment 15•12 years ago
|
||
From my understanding of the bug, 'mark as' should set to read from Comment 0. With the current code, it checks only the first line which by default when clicking will be marked as read so everything becomes unread then you would have to click it once more to mark the entire selection as read.
A scenario:
In the mail system, you have 10 messages in your inbox with the following statuses:
[ ] Read
[ ] Read
[ ] Read
[ ] Unread
[ ] Unread
[ ] Unread
[ ] Unread
[ ] Unread
[ ] Unread
[ ] Unread
STEP 1:
You want to select the last 5 and mark as read so you select them by clicking the top-most and then shift clicking the bottom. It will become like this:
[ ] Read
[ ] Read
[ ] Read
[ ] Unread
[ ] Unread
[x] Read <- Unread message becomes read on click
[x] Unread
[x] Unread
[x] Unread
[x] Unread
STEP 2:
Now you click 'Mark' button
[ ] Read
[ ] Read
[ ] Read
[ ] Unread
[ ] Unread
[x] Unread <- This becomes unread
[x] Unread
[x] Unread
[x] Unread
[x] Unread
STEP: 3
You click 'Mark' button again
[ ] Read
[ ] Read
[ ] Read
[ ] Unread
[ ] Unread
[x] Read
[x] Read
[x] Read
[x] Read
[x] Read
If that's the case, the first selected message should be ignored if there's more than 1 message selected as first message will always becomes read. Then if only one message is selected, we will only check to see if the message is read or not (returns true if read else returns false). So instead of going from Steps 1, 2, and 3, it's Steps 1 and 3.
Comment 16•12 years ago
|
||
Comment on attachment 718262 [details] [diff] [review]
First patch fix submission
Thanks for the explanation. I'm still not sure if it's the Right Thing to do (IanN will have to decide that, since he's been selected as reviewer) but at least I'm satisfied that it's intentional.
Attachment #718262 -
Flags: feedback-
Reporter | ||
Comment 17•12 years ago
|
||
Hi Jenifer!
Please join us on IRC so that we can discuss this further. We are on the Mozilla IRC server in the SeaMonkey dev channel (irc://moznet/seamonkey).
Assignee | ||
Comment 18•12 years ago
|
||
Hello!
Should "Mark as Unread" be added into SeaMonkey like it has been done in Thunderbird? The current patch assumes that there is only one Mark button which can either be Read or Unread depending on what is being selected.
Link to open discussion:
https://groups.google.com/forum/#!topic/mozilla.dev.apps.seamonkey/FKitb0AwuME
Comment 19•12 years ago
|
||
Comment on attachment 718262 [details] [diff] [review]
First patch fix submission
>+++ b/suite/mailnews/mailWindowOverlay.js
Please use 2 spaces for each indentation not 4.
> function SelectedMessagesAreRead()
> {
>+ var thisSelectedMessage;
>+ if (gFolderDisplay.selectedMessages.length == 1) {
>+ thisSelectedMessage = gFolderDisplay.selectedMessages[0];
>+ if (!thisSelectedMessage.isRead)
>+ return false;
> }
I see no reason to special case a single message.
>+ for (let i = 1; i < gFolderDisplay.selectedMessages.length; ++i) {
so you might as well start with i = 0. As the number of selected messages is usually not too high there is probably no benefit from getting the selection count before the for loop, but I'd not object to it either.
>+ thisSelectedMessage = gFolderDisplay.selectedMessages[i];
>+ if (!thisSelectedMessage.isRead)
Just inline gFolderDisplay.selectedMessages[i]
>+ return false;
> }
>+ return true;
> }
f+ for the moment as I would like to see the new patch.
Attachment #718262 -
Flags: review?(iann_bugzilla) → feedback+
Assignee | ||
Comment 20•12 years ago
|
||
Attachment #724281 -
Flags: review?(iann_bugzilla)
Attachment #724281 -
Flags: review?(iann_bugzilla) → review+
Attachment #718262 -
Attachment is obsolete: true
Comment 21•12 years ago
|
||
Comment on attachment 724281 [details] [diff] [review]
Changed the patch in response to first feedback [Checked in: Comment 21]
http://hg.mozilla.org/comm-central/rev/88e9e61f5506
Attachment #724281 -
Attachment description: Changed the patch in response to first feedback → Changed the patch in response to first feedback [Checked in: Comment 21]
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.19
You need to log in
before you can comment on or make changes to this bug.
Description
•