Closed
Bug 572065
Opened 14 years ago
Closed 14 years ago
Attachment functions broken - open/save/detach don't work (IMAP only)
Categories
(MailNews Core :: Networking: IMAP, defect)
MailNews Core
Networking: IMAP
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: u331436, Assigned: Bienvenu)
References
Details
(Keywords: regression, Whiteboard: [tbtrunkneeds])
Attachments
(3 files, 2 obsolete files)
1.93 KB,
patch
|
Bienvenu
:
review+
Bienvenu
:
superreview+
|
Details | Diff | Splinter Review |
1.12 KB,
patch
|
Details | Diff | Splinter Review | |
9.43 KB,
patch
|
rkent
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a6pre) Gecko/20100614 Lightning/1.1a1pre SeaMonkey/2.0pre - not Firefox Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a6pre) Gecko/20100614 Lightning/1.1a1pre SeaMonkey/2.0pre - not Firefox unable to open or save attachments to mail. Message display one or several attachment. Double clicking on it to open or "save as" not working. workaround: forward message "inline", the new message opens and then it is possible to double-click to open the attachment or "save as".. Seems a regression since about Friday 11th or Saturday 12th of June. Will try from a test profile (no add-on) Reproducible: Always Steps to Reproduce: 1.double-click and attachment 2 [review].expect the registered application to open (word, excel, adobe...) 3.nothing happens Alternatively, 1. "save as" open up the dialog to save the attachment 2 [review]. Type in name and location of file to save 3. Nothing happens Expected Results: Application should open / file should be save in chosen location Will try ASAP with a test profile
Test profile shows the same behavior. - unable to open an attachment using the default applications - unable to save as a file, in any chosen location It seems that the download do not start ! (My email account use IMAP with all message downloaded locally (main profile) or staying on the mail server (test profile)).
Comment 2•14 years ago
|
||
See also bug 572086. Attachments open and save for me if they are in Local Folders, but not IMAP. So you can copy the affected message into Local Folders/Trash and then open and/or save the attachment. Attachments open for me if they are readable by the browser (text, PDF etc.) but I still can't save them directly from IMAP. Another workaround: right-click, View Source, then use the Save As option.
Comment 3•14 years ago
|
||
Confirmed for Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US; rv:1.9.3a6pre) Gecko/20100615 Lightning/1.1a1pre SeaMonkey/2.1a2pre > Attachments open and save for me if they are in Local Folders, but not IMAP. So > you can copy the affected message into Local Folders/Trash and then open and/or > save the attachment. > Attachments open for me if they are readable by the browser (text, PDF etc.) > but I still can't save them directly from IMAP. Same here. IMAP account FAIL. Local folder WFM.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 4•14 years ago
|
||
Setting dependency for now since Bug 572086 is in a different product.
Depends on: 572086
Comment 5•14 years ago
|
||
(In reply to comment #4) > Setting dependency for now since Bug 572086 is in a different product. I think its fairly safe to say these are the same issue, therefore moving product and merging etc.
Component: MailNews: Message Display → Networking: IMAP
No longer depends on: 572086
Keywords: regression,
regressionwindow-wanted
OS: Windows XP → All
Product: SeaMonkey → MailNews Core
QA Contact: message-display → networking.imap
Hardware: x86 → All
Summary: unable to open or save message attachment → Attachment functions broken - open/save/detach don't work (IMAP only)
Whiteboard: [tb32needs]
Saving of an attachment from an unsynchronized IMAP folder still works for me on trunk with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a5pre) Gecko/20100608 Shredder/3.2a1pre; I didn't set any mime_parts_on_demand prefs for this profile, in case that makes a difference.
Dominique in comment 0 was spot on with the range. Regression range: Works: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a5pre) Gecko/20100610 Shredder/3.2a1pre 2010-06-10-04-comm-central-trunk Broken: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a6pre) Gecko/20100611 Shredder/3.2a1pre 2010-06-11-03-comm-central-trunk Pushlog - Comm-central: http://hg.mozilla.org/comm-central/pushloghtml?startdate=2010-06-10+03%3A00%3A00&enddate=2010-06-11+04%3A00%3A00 Pushlog - Mozilla-central: http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2010-06-10+03%3A00%3A00&enddate=2010-06-11+04%3A00%3A00
Keywords: regressionwindow-wanted
Comment 9•14 years ago
|
||
Wild guess: neil@parkwaycc.co.uk Thu Jun 10 03:10:37 2010 -0700 c2c24182c931 Jan Horak — Bug 377319 Convert mailnews/imap to the external API r=Neil sr=bienvenu 49782a242a72 Jan Horak — Bug 377319 Convert mailnews/base/src to the external API r=Neil sr=bienvenu 12981f9325ed Jan Horak — Bug 377319 Convert mailnews/mime to the external API r=Neil sr=bienvenu
Comment 10•14 years ago
|
||
Typo fix aside, I also fixed MsgFindCharInSet, the code is loosely based on nsStringObsolete.cpp's version of FindCharInSet.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #451542 -
Flags: superreview?(bienvenu)
Attachment #451542 -
Flags: review?(bugzilla)
Assignee | ||
Updated•14 years ago
|
Attachment #451542 -
Flags: superreview?(bienvenu) → superreview+
Assignee | ||
Comment 11•14 years ago
|
||
Comment on attachment 451542 [details] [diff] [review] Proposed patch - checked in. this does fix the problem, and the code is based on the nsStringObsolete code, as asserted...
Comment 12•14 years ago
|
||
Comment on attachment 451542 [details] [diff] [review] Proposed patch - checked in. This seems to fix my open problem, but not the saving or detaching of attachments. I also think we should be thinking about a unit test for at least one of these scenarios.
Updated•14 years ago
|
Assignee: neil → nobody
Status: ASSIGNED → NEW
Reporter | ||
Comment 13•14 years ago
|
||
Is there anything blocking resolution? Any chance or getting this bug solved ? (Sorry, I do not intend to be rude or too pressing on the developers.. my apologies for my own impatience).
Assignee | ||
Comment 15•14 years ago
|
||
This is next on my list of things to do.
Assignee: nobody → bienvenu
Assignee | ||
Comment 16•14 years ago
|
||
if (fetchService) { PRInt32 sectionPos = urlString.Find("?section"); fullMessageUri.Append(Substring(urlString, sectionPos)); } vs previous. if (fetchService) { nsCString mimePart; PRInt32 sectionPos = urlString.Find("?section"); urlString.Right(mimePart, urlString.Length() - sectionPos); fullMessageUri.Append(mimePart); } If sectionPos is -1, the new code does not do the same thing as the old code. Granted, the old code produced a rather strange uri, but it's what nsParseImapMessageURI expects. I'll fix it, and try to write a unit test for saving imap attachments.
Assignee | ||
Comment 17•14 years ago
|
||
Substring(urlString, -1) returns the empty string, I believe.
Assignee | ||
Comment 18•14 years ago
|
||
Along with your previous patch, this patch makes save as and detach work for imap. I'm working on a unit test now. Once we have a unit test, we can maybe improve the code, but w/o one, I'd rather make it work the way it used to...
Assignee | ||
Comment 19•14 years ago
|
||
I'm really asking rkent for a review of the test, and neil for r/sr for the fix itself.
Attachment #453963 -
Flags: superreview?(neil)
Attachment #453963 -
Flags: review?(kent)
Comment 20•14 years ago
|
||
Comment on attachment 453963 [details] [diff] [review] fix with unit test > PRInt32 sectionPos = urlString.Find("?section"); >- fullMessageUri.Append(Substring(urlString, sectionPos)); >+ if (sectionPos == kNotFound) >+ fullMessageUri.Append(urlString); >+ else >+ fullMessageUri.Append(Substring(urlString, sectionPos)); Could it be that the IMAP code just needs the query part of the string, whether or not it contains section, i.e. call FindChar('?') instead?
Comment 21•14 years ago
|
||
Comment on attachment 451542 [details] [diff] [review] Proposed patch - checked in. Shouldn't the review request here be cancelled?
Assignee | ||
Comment 22•14 years ago
|
||
No, we need that patch too, I believe, right, Neil? I think we look for section later on, but perhaps not. I'll look.
Comment 23•14 years ago
|
||
(In reply to comment #22) > No, we need that patch too, I believe, right, Neil? Yes, we need that for opening attachments. > I think we look for section later on, but perhaps not. I'll look. Obviously if it is ?section then it doesn't matter whether we look for ? or ?section but in the non-?section case then just looking for ? should work.
Assignee | ||
Comment 24•14 years ago
|
||
per Neil's suggestion of looking for '?'
Attachment #453963 -
Attachment is obsolete: true
Attachment #454149 -
Flags: superreview?(neil)
Attachment #454149 -
Flags: review?(kent)
Attachment #453963 -
Flags: superreview?(neil)
Attachment #453963 -
Flags: review?(kent)
Comment 25•14 years ago
|
||
Comment on attachment 454149 [details] [diff] [review] fix addressing Neil's comment Doesn't hurt to check that the ? exists, I guess, just in case someone passes in a non-attachment ;-)
Attachment #454149 -
Flags: superreview?(neil) → superreview+
Assignee | ||
Comment 26•14 years ago
|
||
Comment on attachment 451542 [details] [diff] [review] Proposed patch - checked in. given that we (will soon) have a test for this, I'm going to steal the review from standard8 and r/sr+ it.
Attachment #451542 -
Flags: review?(bugzilla) → review+
Comment 27•14 years ago
|
||
The unit test is hanging for me (Windows 7), with the last log message "Starting test: startDetach" I tried a few other tests to make sure that my setup is working OK, and they worked fine. I've mentioned this before, but if you just wait the test eventually times out, and then claims that it "passed". There is something rotten somewhere in the testing infrastructure that a timeout is a pass.
Assignee | ||
Comment 28•14 years ago
|
||
rkent, does this hang for you? I've seen the same thing with tests passing on timeout - I wonder if it's an issue with the async_driver framework stuff, or something more basic.
Attachment #454149 -
Attachment is obsolete: true
Attachment #454280 -
Flags: review?(kent)
Attachment #454149 -
Flags: review?(kent)
Comment 29•14 years ago
|
||
The test still times out. Your change was in the wrong place anyway, it never gets past the yield at the end of startDetach. I don't have time now to confirm this, but what I think I see in the debugger is that this code in nsParseImapMessageURI: if (part && keyEndSeparator != -1) { PRInt32 partPos = MsgFind(uriStr, "part=", PR_FALSE, keyEndSeparator); if (partPos != -1) { *part = ToNewCString(Substring(uriStr, keyEndSeparator)); } } is not finding the part when uriStr is: + uriStr {mStorage=0x0014c5c0 "imap-message://user@localhost/INBOX#1?part=1.2&filename=bob.txt" } nsCAutoString which I don't understand. Maybe I'm completely offtrack though. I'll try to get to this again soon.
Assignee | ||
Comment 30•14 years ago
|
||
rkent, did you apply Neil's first patch? It's also required.
Comment 31•14 years ago
|
||
No, I did not. I'll try applying https://bugzilla.mozilla.org/attachment.cgi?id=451542
Comment 32•14 years ago
|
||
With Neil's patch applied, the test is now running normally. Sorry about that. I'll now review the code itself.
Comment 33•14 years ago
|
||
Comment on attachment 454280 [details] [diff] [review] fix timeout function - checked in. Nit: I could not find any global definition of gMessage, so I think it should be just "let message =" and then use message instead of gMessage. I don't really understand the requirement for dummyWindow, though I did confirm the test fails without it. Is this an issue with detachAttachmentsWOPrompts that should be fixed?
Attachment #454280 -
Flags: review?(kent) → review+
Assignee | ||
Comment 34•14 years ago
|
||
(In reply to comment #33) > I don't really understand the requirement for dummyWindow, though I did confirm > the test fails without it. Is this an issue with detachAttachmentsWOPrompts > that should be fixed? Yes, the detach code in general requires that the folder be open in a message window, so that after doing the append of the new copy of the message, we update the folder, and download the header for the newly appended message, which triggers the detach code to continue. I've expanded the comment in the test code to try to explain this a little better, and I'll file a bug on detachAttachmentsWOPrompts.
Assignee | ||
Updated•14 years ago
|
Flags: in-testsuite+
Assignee | ||
Updated•14 years ago
|
Attachment #451542 -
Attachment description: Proposed patch → Proposed patch - checked in.
Assignee | ||
Updated•14 years ago
|
Attachment #454280 -
Attachment description: fix timeout function → fix timeout function - checked in.
Assignee | ||
Comment 35•14 years ago
|
||
both patches checked in (sorry, Neil, forgot to put bug # when I pushed your patch :-( )
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 36•14 years ago
|
||
Verified fixed with IMAP account and today's nightly: 20100629010301 (Win)
Status: RESOLVED → VERIFIED
Comment 37•14 years ago
|
||
Pushed on the SeaMonkey 2.1a2 relbranch as http://hg.mozilla.org/comm-central/rev/c2d2f31db138 and http://hg.mozilla.org/comm-central/rev/238198843d19 so this is fixed there (in build2) as well.
Comment 38•14 years ago
|
||
This problem occurs with imap and pop3 account settings.
Updated•14 years ago
|
Whiteboard: [tb32needs] → [tbtrunkneeds]
Comment 41•14 years ago
|
||
With Thunderbird 3.1.4 and CompactHeader 1.3.0beta3 installed, it is not possible to open some attachment (not completely reproducible, seems to be correlated to the attachment type but also to the specific computer/OS). After the update to Thunderbird 3.1.5 that problem disappeared. Is it related to this bug or is there another bug description at bugzilla which fixed it?
You need to log in
before you can comment on or make changes to this bug.
Description
•