Tags are lost on messages with attachment in IMAP (when the server doesn't supports any keywords)

RESOLVED FIXED in Thunderbird 3.0b3

Status

MailNews Core
Backend
--
critical
RESOLVED FIXED
11 years ago
9 years ago

People

(Reporter: Satya, Assigned: Bienvenu)

Tracking

(Blocks: 1 bug, {dataloss})

unspecified
Thunderbird 3.0b3
dataloss
Bug Flags:
blocking-thunderbird3 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

135.62 KB, text/plain
Details
13.38 KB, patch
standard8
: review+
neil@parkwaycc.co.uk
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

11 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.1) Gecko/20061208 Firefox/2.0.0.1
Build Identifier: Thunderbird/2.0b2 2007011615

I have some messages on Scalix IMAP (I don't know if it supports keywords). Tags on messages with attachments in the IMAP folders are lost on the next refresh.

Reproducible: Always

Steps to Reproduce:
1. Tag some messages with attachments on IMAP
2. Wait for refresh or manually refresh

Actual Results:  
Tags were gone

Expected Results:  
Tags should be preserved

Comment 1

11 years ago
Try creating an imap protocol log http://www.mozilla.org/quality/mailnews/mail-troubleshoot.html#imap

Comment 2

11 years ago
My Thunderbird (Version 2.0.0.4 (20070604)) lost the tag(s) in a message when I delete an attachment in this message...

Comment 3

10 years ago
I appear to be experiencing the same issue.  

My steps to reproduce are:
1. Create a Saved Search folder that searches an IMAP server for a certain tag
2. Tag a message with an attachment
3 [review]. Go to the Saved Search folder.  The message will appear.
4. Return to the actual IMAP folder.  The message will no longer have the tag associated with it.

This is with Thunderbird 2.0.0.9 (Windows/20071031) and a new profile.  My platform is Windows XP.  The remote IMAP server is at Everyone.net; I'm not sure what software they use.

I'm attaching an IMAP log, but I'm not sure how much help it is (I don't see anything obviously wrong).  Let me know if there's any other information I can provide.

Comment 4

10 years ago
Created attachment 295863 [details]
IMAP log

Updated

10 years ago
Assignee: mscott → nobody
Severity: normal → critical
Keywords: dataloss, qawanted

Comment 5

10 years ago
The server says it doesn't support labels of any kind. (Not even the few predefined.)
Blocks: 432710
Magnus: so does this mean tags aren't functional on that sort of server? If so, shouldn't we change the UI to prevent people using them?

Gerv

Comment 7

10 years ago
The tags will usually still work - but on the local machine only. (There's special code for that tries to handle this.)

Comment 8

10 years ago
similar to bug 411869 - warn if server doesn't supported feature like marking message with star.  In that case, message gets starred in UI, click away and then back to folder then stars are gone.
Component: General → MailNews: Backend
Product: Thunderbird → Core
QA Contact: general → backend

Comment 9

10 years ago
OK, so it looks like it's the fault of the server for not keeping tags.  But then why can I set tags on messages that don't have an attachment, and they stick just fine?

Presumably if the server can't store the tags, then TB is doing some magic to keep them locally.  But then why doesn't the magic work if there's an attachment?

Even if the tags have to be local, which is sub-optimal, I can live with it as long as they actually persist.
Product: Core → MailNews Core

Comment 10

9 years ago
(In reply to comment #9)
> OK, so it looks like it's the fault of the server for not keeping tags.  But
> then why can I set tags on messages that don't have an attachment, and they
> stick just fine?

perhaps david knows
(Assignee)

Comment 11

9 years ago
I can't think why messages w/ attachments should be treated any differently. Can you try a test and see if this happens on a non-built in tag, i.e., a 6th or 7th tag? We could be hitting code that clears the built in tags (though I kinda doubt it if the server doesn't support keywords at all)

Comment 12

9 years ago
Thanks for your attention, David.  Unfortunately, I just tested it and it makes no difference if it's a built-in tag or one that I added.

Is there anything else I can do to help troubleshoot this?
(Assignee)

Comment 13

9 years ago
the offline playback is generating a bogus url to set the keyword on the server. Working on a patch.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Keywords: qawanted
Whiteboard: investigating

Comment 14

9 years ago
requesting blocking based on dataloss and prospects for patch
Flags: blocking-thunderbird3?

Comment 15

9 years ago
By the way, if it helps for purposes of getting the blocking flag set, I can confirm that this bug occurs on other platforms than x86 Linux -- I've consistently confirmed it with Windows XP, for example.  I'm pretty sure the platform field should be "All".

Comment 16

9 years ago
Daniel: can you test if it's still a problem with a build from here?
http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/latest-comm-central/
OS: Linux → All
Hardware: x86 → All

Comment 17

9 years ago
Yes, it's still an issue as of the build "Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090129 Shredder/3.0b2pre".  Steps to reproduce are the same as in Comment #3.

Updated

9 years ago
Status: ASSIGNED → NEW
Summary: Tags are lost on messages with attachment in IMAP → Tags are lost on messages with attachment in IMAP (when the server doesn't supports any keywords)
(Assignee)

Comment 18

9 years ago
So, the server claims not to support any non-default flags, but has a special keyword set when a message has an attachment (X-EON-HAS-ATTACHMENT). We're taking that keyword and crunching the local keywords the user has set. I'm not quite sure how we should handle this - we could just ignore keywords on the server if the server say there aren't any non-default permanent flags, though I can imagine that it might be useful for extensions to get hold of those server-set keywords. Or we could avoid crunching the local keywords if the server says it doesn't store user-defined keywords. But then if the keyword gets removed on the server, we won't have any way of storing that. We could have two properties for headers for this case, one for user set keywords and one for server set keywords, but that makes our implementation more complicated.

Comment 19

9 years ago
(In reply to comment #18)
>
> we could avoid crunching the local keywords if the
> server says it doesn't store user-defined keywords.

I don't understand what is wrong with that solution. Why would an extension want to get the pseudo-keyword added by the server? I guess I don't really understand why the server is adding this bogus flag. (I assume by "has a special keyword set" what you mean is that the server adds another named flag).

If an extension wants the value of the flags from the server, can't it just ask directly?
(Assignee)

Comment 20

9 years ago
(In reply to comment #19)

> I don't understand what is wrong with that solution. Why would an extension
> want to get the pseudo-keyword added by the server? I guess I don't really
> understand why the server is adding this bogus flag. (I assume by "has a
> special keyword set" what you mean is that the server adds another named flag).

Yes, we can't know why a server would add a flag, so we can't know why an extension might want to know about the flag. In enterprises, I've come across server-side processes that add their own keywords to messages, and wanted to have extensions be able to access those keywords.

> 
> If an extension wants the value of the flags from the server, can't it just ask
> directly?

Not synchronously or easily.

All that being said, I'm going to go with a variant of this solution. I'd much rather encourage servers to turn on support for user-set keywords.

Comment 21

9 years ago
I concur with the sentiment of comment #19.  It seems the case where an extension wants to get at the keywords is more of an enhancement (I'm not aware of any extensions that look for this kind of keyword) whereas the case where we crunch the local keywords is a dataloss bug.

Are we really worried that "special" (i.e. non-standard) keywords that were set by the server might change over time?  I doubt that the server is going to remove the X-EON-HAS-ATTACHMENT keyword unexpectedly.  I would think the same is probably true of the other enterprise-specific keywords you mention (although admittedly this is a wild guess).

If I understand the last sentence of comment #18 correctly, that alternative makes sense (storing user-set and server-set keywords separately).  Then when the server says it doesn't support user-set keywords, we don't allow it to make changes to our local user-set keywords, but it can still make changes to server-set keywords.  This seems like the most ideal solution, although it is more complicated for very little practical gain.
(Assignee)

Updated

9 years ago
Status: NEW → ASSIGNED
Whiteboard: investigating → [has patch for review]
(Assignee)

Comment 22

9 years ago
Created attachment 365304 [details] [diff] [review]
proposed fix

This makes HandleCustomFlag take a boolean which says whether the server supports custom keywords. If it doesn't, we still parse for things like $Junk, NonJunk, etc, but we don't crunch the msg hdr keywords property.

I cleaned up nsImapFlagAndUIDState to get rid of an unused constructor, and unused initial flags parameter.
Assignee: nobody → bienvenu
Attachment #365304 - Flags: superreview?(neil)
Attachment #365304 - Flags: review?(bugzilla)

Updated

9 years ago
Attachment #365304 - Flags: superreview?(neil) → superreview-
Comment on attachment 365304 [details] [diff] [review]
proposed fix

>+  /**
>+   * Set of flags the server supports storing per message. See nsImapCore.h
>+   * for the set of flags. Note that the setter OR's the  passed in flags
>+   * with the previous flags because we want to accumulate the FLAGS
>+   * and PERMANENTFLAGS response.
>+   */
>+  attribute unsigned short supportedUserFlags;
IMHO This is unacceptable behaviour for an attribute. Please
a) either make the attribute readonly or change it to getSupportedUserFlags()
and
b) restore setSupportedUserFlags and optionally name it orSupportedUserFlags.

>+                                 userFlags & kImapMsgSupportUserFlag,
PRBool should be either PR_TRUE or PR_FALSE. Please
a) pass in all the flags, and test in HandleCustomFlags
or
b) just change the type of the argument to PRUint16
or
c) use !!( & ) to force the result to a boolean.
(Assignee)

Comment 24

9 years ago
Comment on attachment 365304 [details] [diff] [review]
proposed fix

yeah, you're right - this wouldn't be an interface at all if it weren't for xpcom proxying, but since it is one, we should play by the rules.
Attachment #365304 - Attachment is obsolete: true
Attachment #365304 - Flags: review?(bugzilla)
(Assignee)

Comment 25

9 years ago
Created attachment 365441 [details] [diff] [review]
fix addressing Neil's comments
Attachment #365441 - Flags: superreview?(neil)
Attachment #365441 - Flags: review?(bugzilla)

Updated

9 years ago
Attachment #365441 - Flags: superreview?(neil) → superreview+
Attachment #365441 - Flags: review?(bugzilla) → review+
Comment on attachment 365441 [details] [diff] [review]
fix addressing Neil's comments
(Assignee)

Comment 27

9 years ago
fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Flags: blocking-thunderbird3? → blocking-thunderbird3+
Resolution: --- → FIXED
Whiteboard: [has patch for review]
Target Milestone: --- → Thunderbird 3.0b3
You need to log in before you can comment on or make changes to this bug.