386855 - attachment file name stripped of consecutive spaces

Status: VERIFIED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Pete B]

User-Agent:       Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 5.1; .NET CLR 1.1.4322; .NET CLR 2.0.50727; FDM)
Build Identifier: version 2.0.0.4 (20070604)

Receiving a mail with more than one consecutive space in an attachment filename and then saving to disk strips down the consecutive spaces to a single space.  e.g. the filename "4    spaces.txt" becomes "4 spaces.txt".  This occurs with mails sent from Thunderbird or Outlook.  Sending a mail from thunderbird TO outlook does not appear to have the problem.


Reproducible: Always

Steps to Reproduce:
1. Create a text file with consecutive spaces "4    spaces.txt"
2. Send as an attachment to self
3. Save the attachment and inspect the filename
Actual Results:  
"4    spaces.txt" becomes "4 spaces.txt"


Expected Results:  
Attachment filenames should not be modified.

Originally seen with a 1.5x version.  Upgraded to version 2.0.0.4 (20070604) which still has the bug.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I can see this behavior with Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a6pre) Gecko/20070622 Thunderbird/3.0a1pre ID:0000000000 [cairo]

As it looks like the spaces are getting stripped while the file is attached. If you open the sent folder after the message is sent the attachment only contains one space character.

Cannot find any other bug which handles the same issue.

Comment 2

[Pete B]

Interestingly though - if I send the attachment from thunderbird to a different mail client then the spaces are not stripped off.  Presumably it is down to the code that reads the mail and extracts the attachment file name (I'm assuming here that the sent box reuses the code that the inbox uses for parsing mails)?

Comment 3

[Magnus Melin [:mkmelin]]

Sorry, certainly didn't mean to do *those* changes...

Comment 4

[Robert Kaiser]

I just tested this on Linux with SeaMonkey branch (XUL filepicker), trunk (GTK2 filepicker) and Thunderbird trunk (GTK2 filepicker), and it fails only in Thunderbird, maybe due to the different attachment pane, where it's already differently displayed (SeaMonkey always displays all spaces, Thunderbird only one).

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thanks Robert. Meanwhile I checked my MBOX file and noticed that this isn't a backend problem. It seems that Thunderbird can't display multiple blanks.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment part of the message:

Content-Type: text/plain; x-mac-type="0"; x-mac-creator="0";
 name="test    tz"
Content-Transfer-Encoding: base64
Content-Disposition: inline;
 filename="test    tz"

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: No removal of additional blanks

David, why do we remove multiple blanks from displayName? I know that shortens up the text but it also shows a modified filename. We shouldn't patronize the user.

Comment 8

[Scott MacGregor]

Comment on attachment 271130 [details] [diff] [review]
No removal of additional blanks

This was added intentionally, pleae don't undo it. In the future you can use CVS blame so see why code was added :)

This is for Bug 300246.

Comment 9

[David :Bienvenu]

yeah, what Scott said :-)

Comment 10

[Pete B]

So its not a bug its a feature! :grin:

I can see why it is useful when displaying the filename, but not when the file is stored to disk..  I'm maintaining an application where files can have spaces in the extension - if they are stipped then file filtering does not work correctly.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Patch v2: Only shorten the display name

This patch observes the security issue of bug 300246 and only displays the shortened filename within the attachment pane. The tooltext still shows the full name and you are also able to save the file with the correct name. This is the spun off patch from attachement 271260 which is for MailNews.

Comment 12

[Pete B]

Nice 1 :)

Comment 13

[David :Bienvenu]

Comment on attachment 271275 [details] [diff] [review]
Patch v2: Only shorten the display name

what about things like the attachment menu? Is the display name used to construct the menu? Do we care?

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #13)
> what about things like the attachment menu? Is the display name used to
> construct the menu? Do we care?

Oh, thanks David! I was never in touch with that menu. I'll come up with an updated version.

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Patch v3 also fixes the attachment menu

Ok, this fixes the remaining issue now. If we need to display the attachment name once more we can rely on the new function createAttachmentDisplayName.

Comment 16

[David :Bienvenu]

Comment on attachment 271710 [details] [diff] [review]
Patch v3 also fixes the attachment menu

thx, Henrik.

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I raise the priority because we currently mangle the filename. Would this be a fix we would like to have for branch 1.8? If yes, I think it's too late for 1.8.1.5?

Comment 18

[David :Bienvenu]

it's probably too late for 1.8.1.5, but I think we'd consider it for a future 1.8 branch build. You could nominate it for 1.8.1.6, after it's been checked into the trunk and baked for a while.

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Scott, could you please super-review my patch?

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+2]]

FYI: The appropriate patch for SeaMonkey on bug 300246 was checked in 5 days ago.

Comment 21

[Scott MacGregor]

Comment on attachment 271710 [details] [diff] [review]
Patch v3 also fixes the attachment menu

thanks henrik.

Comment 22

[Karsten Düsterloh]

Landed on trunk.

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Pete, could you test a tomorrows trunk nightly build if it fixes the issue for you? If yes, then use a fresh profile to not destroy your working one. If all is fine I'll ask for 1.8 approval. 

Comment 24

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified with version 3.0a1pre (2007072105).

Comment 25

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Patch v3 for 1.8 Branch

The patch doesn't apply well on 1.8 Branch. Here is the updated version of Patch v3 which is already checked into trunk. Carrying over r/sr because there are made no changes to the code.

Comment 26

[Karsten Düsterloh]

Attachment 273249 [details] [diff] landet on MOZILLA_1_8_BRANCH.

Comment 27

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Works fine for Thunderbird version 2.0.0.7pre (20070913). Now we are on a secure way even on 1.8 branch.