spam e-mail from yahoo imap server marked as junk doesn't keep junk flag
Categories
(MailNews Core :: Networking: IMAP, defect)
Tracking
(Not tracked)
People
(Reporter: Douglas_Peale, Assigned: gds)
References
()
Details
Attachments
(1 file, 4 obsolete files)
9.64 KB,
patch
|
jorgk-bmo
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:45.0) Gecko/20100101 Firefox/45.0 Build ID: 20160309193552 Steps to reproduce: wait for spam to slip by Yahoo's spam filter, then try and mark it as spam Actual results: I click on the dot in the spam column, or right click and select mark -> as junk The spam icon momentarily appears in the spam column, then immediately disappears. Expected results: The spam icon should appear and remain in the spam column. I think this is related to imap since I don't have this problem with another e-mail account using a POP server.
Updated•8 years ago
|
Comment 1•8 years ago
|
||
I have found that the the Yahoo! junk flagging works on the $Junk $NotJunk IMAP flags. I am able to change the junk status report in Thunderbird by setting the flag $NotJunk directly using IMAP. (a store # +flags $NotJunk)
Comment 2•8 years ago
|
||
(Sorry (In reply to Reuben Peterkin from comment #1) > I have found that the the Yahoo! junk flagging works on the $Junk $NotJunk > IMAP flags. I am able to change the junk status report in Thunderbird by > setting the flag $NotJunk directly using IMAP. > (a store # +flags $NotJunk) Sorry should have clarified - that was for removing junk status, I assume to set it would be to set $Junk.
Reporter | ||
Comment 3•8 years ago
|
||
I have never had a problem removing the junk flag, only setting it. It will set, then immediately, with no action from me, unset.
Comment 5•7 years ago
|
||
Another item about junk flags xref Bug 1367443 - Email messages flagged as Junk/spam no longer being automatically sent to Junk folder Bug 1237138 - Marking as not junk does not remove Junk Status
Assignee | ||
Comment 6•7 years ago
|
||
It's something about yahoo causing it. When I click the junk icon on a message in my yahoo inbox, the message gets moved to "bulk mail" folder but the junk icon is gray in "bulk mail". I do see it in color for maybe 1/4 second after I click it before it moves. A couple other accounts I do the same and the junk icon remains colored after it is moved to "junk" folder. I will need to look at what is happening in the imap log so I will leave my "needinfo" flag set.
Comment 7•6 years ago
|
||
(In reply to gene smith from comment #6) > ... > A couple other accounts I do the same and the junk icon remains colored > after it is moved to "junk" folder. > I will need to look at what is happening in the imap log so I will leave my > "needinfo" flag set. I had another look - didn't find any related bug reports
Comment 9•6 years ago
|
||
(In reply to gene smith from comment #6) > It's something about yahoo causing it. When I click the junk icon on a > message in my yahoo inbox, the message gets moved to "bulk mail" folder but > the junk icon is gray in "bulk mail". I do see it in color for maybe 1/4 > second after I click it before it moves. > A couple other accounts I do the same and the junk icon remains colored > after it is moved to "junk" folder. > I will need to look at what is happening in the imap log so I will leave my > "needinfo" flag set. hmm, bug 1489859?
Comment 10•6 years ago
|
||
So we have both http://forums.mozillazine.org/viewtopic.php?f=39&t=3041601 and http://forums.mozillazine.org/viewtopic.php?f=39&t=3038777
Comment 11•5 years ago
|
||
(In reply to gene smith from comment #6)
It's something about yahoo causing it. When I click the junk icon on a
message in my yahoo inbox, the message gets moved to "bulk mail" folder but
the junk icon is gray in "bulk mail". I do see it in color for maybe 1/4
second after I click it before it moves.
A couple other accounts I do the same and the junk icon remains colored
after it is moved to "junk" folder.
I will need to look at what is happening in the imap log so I will leave my
"needinfo" flag set.
Have since found: bug 1489859, bug 1074055, bug 1237138
Assignee | ||
Comment 12•5 years ago
|
||
The problem is that Yahoo defines PERMANENTFLAGS $Junk
and $NotJunk
. Tb assumes, mostly correctly, that the junk flags will be Junk
and NonJunk
like the others imap servers define (e.g., gmail and openwave that I verified).
On yahoo account, since tb doesn't see the expected flags Junk and NonJunk, it just sets the message \Seen when marking the message as spam. On response to the command, tb doesn't see the Junk flag in the response so it immediately clears the spam icon back to gray.
I'm not sure how hard this is to fix or if it should be. Also, not sure if Yahoo is violating a spec or RFC for the flag names it uses. TBD...
Assignee | ||
Comment 13•5 years ago
|
||
My explanation in comment 12 is not quite right. The actual problem with yahoo (and aol, which uses the same server) is that the PERMANENTFLAGS don't allow for the creation of custom tags that tb is hardcoded to produce, "Junk" and "NonJunk". Yahoo's PERMANENTFLAGS and FLAGS (untagged responses to SELECT) contain the values "$Junk" and "$NotJunk", but PERMANENTFLAGS does not contain "\*" that indicates that any custom tag (a.k.a., keyword) can be accepted. So "Junk" or "NonJunk", as always produced by TB can't be accepted.
When the user marks a message as junk or not junk, tb produces a url containing the appropriate string "Junk" or "NonJunk". But right before the flag is about to be sent to the server via IMAP using the STORE command, tb determines that custom flags can't be set for the server and does not attempt to send the STORE command at all. It does go ahead and store the \Seen flag, however. When store of \Seen occurs, the yahoo server usually responds with a untagged FLAG response that contain the string "$NotJunk". This flag causes junk icon for the message just marked to quickly return to gray (not junk) state (assuming the action was to mark the message as junk).
At this time the RFCs don't specify exactly which keywords (tags) should be used to indicate junk and not junk for messages. However, this semi-official document recommends using $Junk and $NotJunk as Yahoo is assuming:
https://www.ietf.org/mail-archive/web/morg/current/msg00441.html (Thanks to Wayne for finding this link.)
Tb sending "Junk" or "NonJunk" are OK as long as the server allows for custom keywords ("\*" in the PERMANENTFLAGS). So instead of changing the several places where the strings "Junk" and "NonJunk" are hard-coded, I propose that at the point where the URL is decoded and tb determines that custom keywords are not allowed, instead of not sending the STORE command at all, instead change the flag value from "Junk" to "$Junk" or from "NonJunk" to "$NotJunk" and go ahead send the IMAP command. Yahoo/Aol accepts the flag when this is done. Other servers that also don't accept custom keywords just return a NO imap response and doesn't cause an error notification in tb.
The real problem with Yahoo/Aol is that the store of the \Seen flag causes the $NotJunk response to occur. Other servers I have observed don't return the $NotJunk flag unless it was explicitly set. Therefore, even if the server doesn't store custom flags, when marking a message tb stores in the .msf file (database) the proper junk status and it doesn't get reset. (Of course, other clients may not see the marked junk/not junk status.)
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 14•5 years ago
|
||
This shows the FLAGS and PERMANENTFLAGS responses from servers that I have test accounts on. The general rule is that if the server supports custom keywords, tb will be able to store hard-coded string "Junk" or "NonJunk" and this bug should not occur.
Assignee | ||
Comment 15•5 years ago
|
||
This shows a 1st cut fix for the problem with yahoo and aol. This is the point where the hard-coded string "Junk" or "NonJunk" is sent via IMAP but the server doesn't accept custom keywords. All this does is change "Junk" to "$Junk" and "NonJunk" to "$NotJunk" which are the new "canonical" strings and they are accepted by Yahoo/Aol servers so the bug is fixed.
I have tested this some with other servers that don't accept custom keywords. Some return a NO response and others just accept the keywords but don't really store them. In both cases, the keywords are still stored locally and retained in the database .msf file.
If custom keywords are accepted, the normal hard-coded keywords are still stored, "Junk" and "NonJunk".
This is not an official patch but just for illustration. Also, just noticed some comment typo's in that at two places it says "Try $Junk" and it should say "Try $NotJunk".
Updated•5 years ago
|
Assignee | ||
Comment 16•5 years ago
|
||
Here's a formal patch for making yahoo's (and AOL's) junk / not junk marking of the trash icon remain in place (stick) instead of possibly changing back to its previous state.
This differs from the preliminary diff (attachment 9040628 [details] [diff] [review]: yahoo.diff) in that the attempt to set the keywords only occurs if the keywords $Junk and $NotJunk are present in the FLAGS response. This will avoid attempting to set the keywords to servers that don't support them and thus avoid the "NO" response.
In addition a small change is made to the Bug 583677 fix since when function parse_folder_flags() is called for FLAGS (and not for PERMANENTFLAGS) the expression (fSupportsUserDefinedFlags & kImapMsgSupportUserFlag)
is always false so no reason to check it. See https://searchfox.org/comm-central/rev/582bd5d57be824ff97c13c7b6e097b9cf666a953/mailnews/imap/src/nsImapServerResponseParser.cpp#1856
There is also a small issue that could occur if parse_folder_flags() is called first for PERMANENTFLAGS and then for FLAGS. In this case the user defined keywords (including $Junk and $NotJunk) won't be stored and this bug will not be fixed (nor will bug 583677). I don't know why the parsing of the FLAGS is skipped in this case. See
https://searchfox.org/comm-central/rev/582bd5d57be824ff97c13c7b6e097b9cf666a953/mailnews/imap/src/nsImapServerResponseParser.cpp#782 However, I have never seen this reverse order occur and have only seen FLAGS first followed by PERMANENTFLAGS. See attachment 9040627 [details]: permanentflags.txt
Comment 17•5 years ago
|
||
Sorry for the delay. I've been on a conference and then side-tracked into other issues. And those IMAP changes aren't easy reviews.
Comment 18•5 years ago
|
||
My sincere apologies for the delay here. We had a few weeks with terrible bustages (still not all fixed) and keeping Daily going and pushing releases out the door is my priority.
Let's start this with a try run to avoid surprises:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=8f302c4cddce12e959b6674526fcd5a9a2db50db
Comment 19•5 years ago
|
||
Comment on attachment 9041108 [details] [diff] [review] 1260059-allow-yahoo-junk-marking-to-stick.patch(v0) Review of attachment 9041108 [details] [diff] [review]: ----------------------------------------------------------------- Looks OK. Sorry once again about the delay. Please address the nits flagged below. ::: mailnews/imap/src/nsImapProtocol.cpp @@ +2887,5 @@ > break; > case nsIImapUrl::nsImapMsgStoreCustomKeywords: > { > + // If the server doesn't support user defined flags, don't usually try > + // to set or reset them. But if this an attempt to set/reset TB's user Grammar. But if this *is* ...? @@ +2889,5 @@ > { > + // If the server doesn't support user defined flags, don't usually try > + // to set or reset them. But if this an attempt to set/reset TB's user > + // defined flags "Junk" or "NonJunk", even if the server doesn't > + // support storing user defined flags, but it does allow storing double space |support..storing| @@ +2892,5 @@ > + // defined flags "Junk" or "NonJunk", even if the server doesn't > + // support storing user defined flags, but it does allow storing > + // standard flag names "$Junk" and "$NotJunk", change "Junk" and > + // "NonJunk" to these values respectively, and go ahead and store the > + // modified flags. (Flags are also referred to as keywords or tags.) Maybe you could restructure this sentence, it's a little twisted. @@ +2898,2 @@ > GetSupportedUserFlags(&userFlags); > + bool userDefinedSettable = (userFlags & kImapMsgSupportUserFlag); Lose parenthesis. @@ +2911,5 @@ > + if (!userDefinedSettable) > + { > + if (stdJunkOk) > + { > + if (!PL_strcasecmp(addFlags.get(), "junk")) Please use addFlag.EqualsIgnoreCase("junk") or .Equals(xxx, nsCaseInsensitiveCStringComparator()); Same here and below. ::: mailnews/imap/src/nsImapServerResponseParser.cpp @@ +1852,5 @@ > } > > void nsImapServerResponseParser::parse_folder_flags(bool calledForFlags) > { > + uint16_t labelFlags = 0, junkNotJunkFlags = 0; one variable per line, no commas. @@ +1853,5 @@ > > void nsImapServerResponseParser::parse_folder_flags(bool calledForFlags) > { > + uint16_t labelFlags = 0, junkNotJunkFlags = 0; > + bool storeUserFlags = calledForFlags && fFlagState; Double space after = @@ +1906,4 @@ > > + // Store user keywords defined for mailbox, usually by other clients. > + // But only do this for FLAGS response, not PERMANENTFLAGS response. > + // This is only needed if '\*' does not appeared in a PERMANENTFLAGS Trailing spaces. @@ +1907,5 @@ > + // Store user keywords defined for mailbox, usually by other clients. > + // But only do this for FLAGS response, not PERMANENTFLAGS response. > + // This is only needed if '\*' does not appeared in a PERMANENTFLAGS > + // response indicating the user defined keywords not allowed; but this > + // is not known until this function called for PERMANENTFLAGS which And here.
Assignee | ||
Comment 20•5 years ago
|
||
Jorg, This addresses all your review items, built and retested.
Comment 21•5 years ago
|
||
Comment on attachment 9047608 [details] [diff] [review] 1260059-allow-yahoo-junk-marking-to-stick-v2.patch Review of attachment 9047608 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the update. Still some nits which I can fix myself. ::: mailnews/imap/src/nsImapProtocol.cpp @@ +2932,5 @@ > + if (!userDefinedSettable) > + { > + if (stdJunkOk) > + { > + if (!PL_strcasecmp(subtractFlags.get(), "junk")) Still some undesirable string comparisons here. ::: mailnews/imap/src/nsImapServerResponseParser.cpp @@ +1907,4 @@ > > + // Store user keywords defined for mailbox, usually by other clients. > + // But only do this for FLAGS response, not PERMANENTFLAGS response. > + // This is only needed if '\*' does not appeared in a PERMANENTFLAGS Nit: Grammar derailed. You changed "has not appeared" to "does not appeared".
Assignee | ||
Comment 22•5 years ago
|
||
Should be OK now. I wanted to test it again after changing the string compares I missed. Also, changed the wording some in the comment containing "...does not appeared in a PERMANENTFLAGS response..."
Comment 23•5 years ago
|
||
Comment on attachment 9047915 [details] [diff] [review] 1260059-allow-yahoo-junk-marking-to-stick-v3.patch Thanks indeed, Gene!
Comment 24•5 years ago
|
||
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/85404debffde
Allow setting/resetting junk marking by user for yahoo/aol to stick. r=jorgk DONTBUILD
Updated•5 years ago
|
Description
•