Closed
Bug 114379
Opened 23 years ago
Closed 23 years ago
Labels are lost when moving to another folder (example: delete to trash)
Categories
(MailNews Core :: Backend, defect, P2)
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla0.9.9
People
(Reporter: scottputterman, Assigned: Bienvenu)
References
Details
Attachments
(2 files, 1 obsolete file)
10.63 KB,
patch
|
naving
:
review+
sspitzer
:
superreview+
|
Details | Diff | Splinter Review |
52.02 KB,
text/plain
|
Details |
Using 12/8 build on Win2000. This is my IMAP account. I haven't tried local messages. Labels work in my Inbox. But the following cases don't work: 1. A message is filtered to a different folder. If there was a label filter rule before the move to another folder, the label doesn't show up. 2. Deleting a message with a label. When I look in the trash, the label is gone. 3. Moving/copying a message withe a label loses the label.
Assignee | ||
Comment 1•23 years ago
|
||
I thought Laurel filed a bug on this, but I don't see it anywhere. Anyway, I guess I'll look at it.
Assignee: ssu → bienvenu
bug 110824 was the bug filed by laurel. marked works for me.
Assignee | ||
Comment 3•23 years ago
|
||
it seems to work for local messages, but not for imap. And unfortunately, it won't be easy to get it to work for imap since we're not storing the labels on the server, and when you move a message, we don't copy the msg hdr; we just download it when we select the destination folder.
Assignee | ||
Comment 4•23 years ago
|
||
Scott, I'm pretty sure we decided we weren't going to store the labels on the imap server. Though it would be easy enough to do, we cut it at the design phase. So I'm guessing this should be a wont fix.
Reporter | ||
Comment 5•23 years ago
|
||
I'd rather future this than wontfix it since I think it's an issue that makes things less useable on IMAP. I actually have different labels set up at home and at work. So I don't necessarily want the label stored on the server. I think what I would like is if when you open a folder, the label filters were run on any new messages in that folder. I have no idea if that's possible, but that's the behavior I'd like to see. That way all moves, deletes, downloading on a different computer, etc would do the right thing.
Reporter | ||
Comment 6•23 years ago
|
||
I saw another behavior that seemed like a bug with IMAP labels so I'm mentioning it here because I'm guessing it's the same. If not, I'll file a separate bug. I deleted a message that had a label. I then used undo to bring the message back. It no longer had the label. I'd have expected it to have the label.
Assignee | ||
Comment 7•23 years ago
|
||
right, same problem - since undo delete is just removing the deleted flag on the msg with that key on the server, and the labels aren't stored on the server, we lose the labels. We can't keep the hdrs in the undo queue around for obvious reasons. I'm a little confused about the resolution of future instead of won't fix. If you want separate labels at home and work, then we can't/won't fix this.
Reporter | ||
Comment 8•23 years ago
|
||
I was trying to say that I don't want to wontfix this because I think it should be fixed at some point. If we are unable to fix this because it's too hard or because we run out of time then I would move this past the 1.0 milestone range. Is this a lot of work to fix? Jennifer, what's your opinion on the behavior of labels? Should users be allowed to have different labels for the same mail account on two different machines?
Assignee | ||
Comment 9•23 years ago
|
||
It can't be fixed given the imap4 protocol the way it is today, without storing the labels on the server. When we move/copy a message to a folder, we have no idea what uid/key that message will be in the destination folder, and thus can't attach a label to the header the next time we connect to that folder at some point in the unknown future and download the header. We could save the message id and subject and sender and message size for pending labels and whenever we download a header, look through our pending labels and do a comparison, but that's a lot of work that's not justified, imo, and also not guaranteed to be correct either...
Assignee | ||
Comment 10•23 years ago
|
||
I talked to Scott about this over AIM and convinced him we should store labels on the server, with each message. However, after, I remembered one of the reasons people didn't want to do this, which is that we would store the label # on the server, and what would get displayed on your machine would be the coresponding label color and text for that machine, so if you have a different meaning for labels at home and at work, they would appear differently depending on which machine you used. We have to store the label # and not the actual label text on the server, because of the way we designed and implemented labels (especially, associating a color with a label #, and limiting the number of labels. If we'd just let labels be completely up to the user, then we could store the actual label on the server, but we didn't do that because the approach we took made the feature much easier to use). So, the choice comes down to the tradeoff between having labels span machines and folders, or neither. Personally, I'd want the same labels to appear at home and at work, which is the imap spirit of things in general.
Comment 11•23 years ago
|
||
I would expect labels to work just like a Flag or Unread/Read status, or Priority, etc. It sticks with the message regardless of whether you are at home, work, etc. When i move a message with a Flag, Unread/Read status, Priority, etc., I expect that info to move with the message.
Assignee | ||
Comment 12•23 years ago
|
||
right, and all those attributes are stored on the imap server, in the imap case. Labels are slightly different, however, in the sense that we have this level of indirection that doesn't exist for read/flagged/priority...
Reporter | ||
Comment 13•23 years ago
|
||
David convinced me. Havin my labels dissapear depending on what I do to the message isn't a good thing. So, adding nsbeta1+ and 0.9.9 milestone due to performance work. If it got in earlier, I wouldn't complain :)
Comment 14•23 years ago
|
||
*** Bug 114844 has been marked as a duplicate of this bug. ***
Comment 15•23 years ago
|
||
Because loosing labels on an IMAP server is (hopefully) unacceptable, I see there are currently two choices: 1. Store only the label number on the server and risk seeing the wrong color/description for a message when on another computer. 2. Store the label color and description on the server with the message and always see the message labeled as you intended. It seems that #2 would be the superior solution. The only argument presented againt it was in comment #10 "... because the approach we took made the feature much easier to use)". How much more difficult is it to "use" labels that have a color and description, but *no* number? Maybe this issue should be discussed a bit more before we take the *easier* route and regret it later. OTOH, if at work green labels mean "proposal" and at home they mean "friends", then I might get confused when I have both on my IMAP server at the same time. It's a dilema... Probably, the ID by label number only is the best solution/compromize after all - your supposed to keep work and play separate anyhow. ;)
Assignee | ||
Comment 16•23 years ago
|
||
Peter, your suggestion has the unfortunate problem that if the user wants to change the color/label in the prefs, we'll have to go open every imap folder on the server, go through each message and change the color and description. I don't think that's going to happen.
Comment 17•23 years ago
|
||
Originally reported as 110824 closing that one, leaving this one stand.
QA Contact: esther → laurel
Comment 18•23 years ago
|
||
*** Bug 115633 has been marked as a duplicate of this bug. ***
Comment 19•23 years ago
|
||
*** Bug 117991 has been marked as a duplicate of this bug. ***
Reporter | ||
Updated•23 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P2
Comment 20•23 years ago
|
||
adding example to summary to make this bug easier to find for queries (to avoid dups).
Summary: Labels are lost when moving to another folder → Labels are lost when moving to another folder (example: delete to trash)
Assignee | ||
Comment 21•23 years ago
|
||
I'm going to work on this soon, and just store the label number on the server. As I said before it's the way IMAP works - you get the same view at work and at home. And as Peter said, you should keep your work and play separate! For me (and I think, for a lot of people), they're in completely different accounts. We should probably consider having different labels for different accounts, or having more labels, so users could segregate accounts by using different label values.
Assignee | ||
Comment 22•23 years ago
|
||
store labels on the imap server, using keywords
Assignee | ||
Comment 23•23 years ago
|
||
OK, I basically have this working - labels are stored on the imap server, and retrieved from the server. I still have one bug to work out, and I haven't tested the filter label code yet. But there's one problem that I don't know the solution to: If the imap server says that a message doesn't have a label, I *should* turn off any label in the local db, for the scenario where the user turns off a label at work, and expects it to be turned off at home. However, this will wipe out all the labels that people have built up over the past weeks because none of those labels were stored on the server. I'm not sure how to fix this - I could write one-time code to upload all labels to the server, but I'm not crazy about writing that code.
Assignee | ||
Comment 24•23 years ago
|
||
another possible fix for the problem of clearing labels without crunching everyone's existing labels is to set a special label value when the user clears a label. In that case, I would then clear the label locally if we see that special label value. I'm not excited about that solution, though, because it would go forward forever, for this one time upgrade problem. I'm wondering how upset people would be (Hi, Scott!) if I blew away all the labels...you could get them back by a one time hack: marking all your messages unread, removing your Inbox.msf file, select your inbox, and thereby rerun all your filters. Also, I should mention that the above patch needs some work - there's some duplicated code in the imap protocol that should be moved into a helper function
Reporter | ||
Comment 25•23 years ago
|
||
I wouldn't be upset if my labels were blown away because they are so messed up now anyway that they aren't too useful. I don't know how others would feel, but if this is the best solution, it's better to do it now before the Netscape Beta or Mozilla 1.0 ship. Will this only affect IMAP labels? I'd just post a message in n.p.m.mail-news with the workaround for getting your labels backs when you do this.
Assignee | ||
Comment 26•23 years ago
|
||
yes, only imap labels. OK, I can post a message to that effect, though it will only help users who've set their labels with filters (Hi, Scott!).
Assignee | ||
Comment 27•23 years ago
|
||
the remaining issue is clearing the old labels on the imap server when the user sets a new label (or sets the label to none). This means removing the old keywords. I'm probably going to do this in the view code that sets the label because the imap code that sets labels doesn't know what the current flags/keywords are, unfortunately, and the view code can be changed to remember what the old labels, if any.
Assignee | ||
Comment 28•23 years ago
|
||
this handles the user changing labels by only paying attention to the last label the user sets, and clearing all labels when the user tries to set the label to none. If I've accidentally included diffs for other bugs, I apologize in advance. requesting review, Navin.
Assignee | ||
Comment 29•23 years ago
|
||
I had to change one line of the above patch, towards the bottom, instead of just + else, I had to do + else if (!flags && !addFlags)// we must be turning off labels, so subtract to make sure we only turned off the flags if the caller didn't pass in any flags, and we're not adding flags, because there are some situations where we get into this code with flags == 0 and addFlags set to true. Only if we're turning off labels to we get here with flags == 0 and addFlags false. I'll add a comment to that effect. + if (userFlags & kImapMsgSupportUserFlag) + { + if ((flags & kImapMsgLabelFlags)) + { + // turn into a number from 1-5 + PRUint32 labelValue = (flags & kImapMsgLabelFlags) >> 9; + flagString.Append("$Label"); + flagString.AppendInt(labelValue); + flagString.Append(" "); + } + else if (!flags && !addFlags)// we must be turning off labels, so subtract them all + flagString.Append("$Label1 $Label2 $Label3 $Label4 $Label5 "); + }
Assignee | ||
Updated•23 years ago
|
Attachment #68644 -
Attachment is obsolete: true
Comment 30•23 years ago
|
||
Comment on attachment 69199 [details] [diff] [review] proposed fix sr=sspitzer once you make those last changes you mentioned.
Attachment #69199 -
Flags: superreview+
Comment 31•23 years ago
|
||
I will review this patch but it is going to take some time. Either I can fix my own bugs or keep reviewing your patches :-( Please bear with me.
Assignee | ||
Comment 32•23 years ago
|
||
thanks, sorry, I had a big back log of stuff because the tree was closed all weekend...
Assignee | ||
Comment 33•23 years ago
|
||
I have a bunch of intermixed changes in imap files, so I need this reviewed before I can check in a bunch of already reviewed stuff, so I appreciate your looking at this!
Comment 34•23 years ago
|
||
Comment on attachment 69199 [details] [diff] [review] proposed fix r=naving
Attachment #69199 -
Flags: review+
Assignee | ||
Comment 35•23 years ago
|
||
fix checked in (for servers that support user keywords - other servers will still have this bug, but we can't do anything about that).
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 36•23 years ago
|
||
This fix has introduced a new problem. I get an alert everytime I label a message that says 'The current command did not succeed. The mail server responded: "Specified set of flags is not valid"' Using Microsoft Exchange as IMAP server.
Assignee | ||
Comment 37•23 years ago
|
||
Pratik, could attach a protocol log of a quick session where you open the inbox, label a message, and shut down (and send the log to me if it contains anything you don't want posted)? I tried to prevent the error you're seeing by noticing if the server supports user keywords or not - I may have messed that up, or Exchange may be reporting that it supports user keywords and doesn't. http://www.mozilla.org/quality/mailnews/mail-troubleshoot.html#imap
Comment 38•23 years ago
|
||
With the 2002021503 build I'm seeing the following error when labeling a message: "The current command did not succeed. The mail server responded: UID STORE failed: Bad flag list". My IMAP server is on Linux, but I don't know exactly which type it is.
Comment 39•23 years ago
|
||
a minimal session log with this error
Assignee | ||
Comment 40•23 years ago
|
||
this has been fixed - http://bugzilla.mozilla.org/show_bug.cgi?id=125736 sorry for the inconvenience
Comment 41•23 years ago
|
||
Generally OK using feb21 commercial trunk: win98, mac OS 10.1, linux rh6.2 OK for filtering within account. OK for move or copy via menu or drag&drop to another folder on same account. OK for delete to trash. OK for undo of move/copy/delete. All the above OK for IMAP or POP. Based on the above scenarios, am marking this bug verified. Awaiting word from bienvenu and/or jglick about what to expect for the situations below. Will log a separate bug for cross-account problems if necessary. Cross account IMAP-->any other account (including Local Folders) loses label. Cross account POP-->POP (including Local Folders) is okay. Cross account POP-->IMAP loses label. Copy News-->any account loses label.
Status: RESOLVED → VERIFIED
Comment 42•23 years ago
|
||
Bug 127141 logged for cross-account issues.
Updated•20 years ago
|
Product: MailNews → Core
Updated•16 years ago
|
Product: Core → MailNews Core
You need to log in
before you can comment on or make changes to this bug.
Description
•