Closed
Bug 736798
Opened 12 years ago
Closed 12 years ago
Opening attached .eml's lists wrong attachments
Categories
(MailNews Core :: MIME, defect)
Tracking
(thunderbird13-)
RESOLVED
FIXED
Thunderbird 14.0
Tracking | Status | |
---|---|---|
thunderbird13 | - | --- |
People
(Reporter: squib, Assigned: squib)
References
(Blocks 1 open bug)
Details
(Keywords: regression)
Attachments
(2 files, 1 obsolete file)
1.46 KB,
message/rfc822
|
Details | |
5.41 KB,
patch
|
squib
:
review+
standard8
:
approval-comm-aurora-
standard8
:
approval-comm-beta-
|
Details | Diff | Splinter Review |
This is fallout from bug 674473 (based on when the error started occurring). Basically, if you have a message like so: root 1.txt message.eml 2.txt and you open message.eml, it shows 1.txt and 2.txt as attachments in the attachment pane. We should fix this.
Assignee | ||
Comment 1•12 years ago
|
||
I tried for a while to write a test for this, but I couldn't figure out how to stream an attached message through XPCShell. Oh well.
Attachment #606935 -
Flags: review?(dbienvenu)
Comment 3•12 years ago
|
||
(In reply to Jim Porter (:squib) from comment #1) > Created attachment 606935 [details] [diff] [review] > Fix this > > I tried for a while to write a test for this, but I couldn't figure out how > to stream an attached message through XPCShell. Oh well. I'll have a crack at figuring out how to do that, but a mozmill test should be straightforward.
Comment 4•12 years ago
|
||
When we load the inner .eml file, we call nsMessenger.LoadUrl with "mailbox:///C:/Users/<user>/AppData/Local/Temp/message-1.eml?number=0&part=1.3&filename=Inner%20message.eml&type=application/x-message-display&filename=Inner%20message.eml" I'm wondering if streaming some variation of that as a message would allow us to test the mime handling of the inner part. Or is that what you tried? I think StreamMessage will add the ?number=0, so you probably wouldn't need that.
Comment 5•12 years ago
|
||
Comment on attachment 606935 [details] [diff] [review] Fix this seems to work fine. I think some sort of unit test would really be helpful here, though, since this isn't the first time we've broken something with .eml file handling, and most developers don't do it often...
Attachment #606935 -
Flags: review?(dbienvenu) → review+
Assignee | ||
Comment 6•12 years ago
|
||
I got sick of trying to make XPCShell handle this, so let's test this via Mozmill. I have a try build in progress here: http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry&rev=da7a5addded8
Assignee: nobody → squibblyflabbetydoo
Attachment #606935 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #607427 -
Flags: review+
Assignee | ||
Comment 7•12 years ago
|
||
One thing of note: this will hide undisplayable multipart/alternative subparts again (e.g. ICS "attachments"), but that's ok, since they weren't actually openable. I have a separate patch to fix that for once and for all, which I'll upload just as soon as I find the bug number again...
Depends on: 736881
Assignee | ||
Comment 8•12 years ago
|
||
(In reply to Jim Porter (:squib) from comment #7) > I have a separate patch to fix that for once and for all, which I'll upload just as soon > as I find the bug number again... That's bug 505024, but it turns out it's somewhat more complicated than it seemed. Given that this is libmime, I suppose I should have expected that.
Assignee | ||
Comment 9•12 years ago
|
||
Oh, and here's another try build, as the first one failed: http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry&rev=a5d7524b24d1 (I thought I'd run a second try build that passed, but I can't find it, so I'll just be safe...)
Assignee | ||
Comment 10•12 years ago
|
||
Checked in: http://hg.mozilla.org/comm-central/rev/f331dbdfd17f
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 14.0
Assignee | ||
Comment 11•12 years ago
|
||
Comment on attachment 607427 [details] [diff] [review] Add Mozmill test [Approval Request Comment] Regression caused by (bug #): bug 674473 User impact if declined: Opening attached .eml files will show bogus attachments in the list (granted, this is probably relatively rare). Testing completed (on c-c, etc.): Successful try build here: http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry&rev=c3f31c6e6be8 Risk to taking this patch (and alternatives if risky): Low; the code change is pretty simple, despite it being in libmime.
Attachment #607427 -
Flags: approval-comm-esr10?
Attachment #607427 -
Flags: approval-comm-beta?
Attachment #607427 -
Flags: approval-comm-aurora?
Updated•12 years ago
|
Attachment #607427 -
Flags: approval-comm-esr10?
Attachment #607427 -
Flags: approval-comm-esr10+
Attachment #607427 -
Flags: approval-comm-beta?
Attachment #607427 -
Flags: approval-comm-beta+
Attachment #607427 -
Flags: approval-comm-aurora?
Attachment #607427 -
Flags: approval-comm-aurora+
Comment 12•12 years ago
|
||
Checked into branches: http://hg.mozilla.org/releases/comm-aurora/rev/f372e5ce68d6 http://hg.mozilla.org/releases/comm-beta/rev/a4c78c9d4606
status-thunderbird12:
--- → fixed
status-thunderbird13:
--- → fixed
Comment 13•12 years ago
|
||
I've had to back this out of branches due to test failures. I tried porting bug 736881 and that didn't fix them locally for me either. Test Failure: SyntheticPartMultiMixed is not defined NEXT ERROR TEST-UNEXPECTED-FAIL | /builds/slave/comm-aurora-macosx-opt-unittest-mozmill/build/mozmill/attachment/test-attachment.js | test-attachment.js::setupModule http://hg.mozilla.org/releases/comm-aurora/rev/dd23e7852b24 http://hg.mozilla.org/releases/comm-beta/rev/7fa1ab94c06b
Comment 14•12 years ago
|
||
Given the orange, and we're at the final beta, we're going to leave fixing this for 12. We'd still like to work out about fixing it for 13, and getting whatever we need landed there.
tracking-thunderbird12:
? → ---
tracking-thunderbird13:
--- → +
Updated•12 years ago
|
Attachment #607427 -
Flags: approval-comm-esr10+
Attachment #607427 -
Flags: approval-comm-beta-
Attachment #607427 -
Flags: approval-comm-beta+
Attachment #607427 -
Flags: approval-comm-aurora-
Attachment #607427 -
Flags: approval-comm-aurora+
Comment 15•12 years ago
|
||
squib, do you have time to work on the backport in the next week or so?
Comment 16•12 years ago
|
||
Unfortunately we've not been able to backport this, and we've now not got time before TB 13. The issue is significant because you'll still be able to get to all attachments.
You need to log in
before you can comment on or make changes to this bug.
Description
•