34051 - use nsIFilePicker instead of nsIFileSpecWithUI in mailnews

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect, P2)

Description

[(not reading, please use [e-mail] instead)]

/mailnews/addrbook/src/nsAddressBook.cpp, line 1301 -- 
nsCOMPtr<nsIFileSpecWithUI> fileSpec(getter_AddRefs(NS_CreateFileSpecWithUI()));
/mailnews/addrbook/src/nsAddressBook.cpp, line 1306 -- rv = 
fileSpec->ChooseInputFile("Open File", nsIFileSpecWithUI::eAllFiles, nsnull, 
nsnull);
/mailnews/addrbook/src/nsAddressBook.h, line 32 -- #include 
"nsIFileSpecWithUI.h"
/mailnews/base/src/nsMessenger.cpp, line 30 -- #include "nsIFileSpecWithUI.h"
/mailnews/base/src/nsMessenger.cpp, line 489 -- nsCOMPtr<nsIFileSpecWithUI> 
fileSpec;
/mailnews/base/src/nsMessenger.cpp, line 542 -- nsIFileSpecWithUI::eAllFiles);
/mailnews/compose/resources/content/MsgComposeCommands.js, line 857 -- 
filePicker = filePicker.QueryInterface(Components.interfaces.nsIFileSpecWithUI); 
/mailnews/import/resources/content/importDialog.js, line 432 -- filePicker = 
filePicker.QueryInterface( Components.interfaces.nsIFileSpecWithUI);
/mailnews/import/resources/content/importDialog.js, line 505 -- filePicker = 
filePicker.QueryInterface( Components.interfaces.nsIFileSpecWithUI);
/mailnews/import/resources/content/importDialog.js, line 596 -- filePicker = 
filePicker.QueryInterface( Components.interfaces.nsIFileSpecWithUI);

pavlov, when will nsIFileSpecWithUI be depricated?

Comment 1

[(not reading, please use [e-mail] instead)]

accepting.  marking m17.

Comment 2

[(not reading, please use [e-mail] instead)]

I saw putterman and jefft talking about this in #mozmail.

Comment 3

[Brian Ryner (not reading)]

nominating for beta3... On Linux, the filepicker dialog used with nsIFilePicker
is completely different than the one used with nsIFileSpecWithUI.  We need these
to be uniform.

Comment 4

[selmer (gone)]

Adding mail2 triage nomination keyword.

Comment 5

[nhottanscp]

Using the interface may cause localization problem.
IQA, please test following dialogs in JA localized build.
* Addressbook, mail import dialog.
* Save attachment dialog.

I checked briefly these dialog with PR2 Japanese on Japanese NT and did not see 
any problem.

Comment 6

[nhottanscp]

I see one problem. My imported Address book (Japanese file name) does not show 
up correctly in the address book UI (list of addressbooks). 
I am not sure if this is related to this bug.

Comment 7

[Stuart Parmenter]

continuing to use nsIFileSpecWithUI will cause l10n bugs, since the unix file
picker used by nsIFileSpecWithUI is not at all l10nable.

Comment 8

[Katsuhiko Momoi]

After considerable discussion in the i18n/mail triage,
this bug is now marked as nsbeta3-.

We would like more information of the following type,
however.

1. Are there specicifc problem in L10n caised by not doing this.
2. If so, we should file a specific bug to fix the problem. 

Comment 9

[Stuart Parmenter]

i'm a bit confused here... how did this bug become an i18n/l10n issue? 
nsIFileSpecWithUI is *going away*.  If mailnews continues to use these
interfaces, I will have to comment out the code that uses them and none of their
file pickers will work.  As I don't really want to do this (since that would
suck), It would be nice to have this fixed.

nsIFileSpecWithUI uses a depriciated interface (nsIFileSpec), and uses
nsIFileWidget which on unix is native and not localizable what-so-ever.

They are both going away, so they should really get fixed :-)

Comment 10

[lchiang]

reassign to tony since these are import dlgs
+ per mail triage

Comment 11

[tonyr]

This is a dup of 39036 for me.  There is a patch/fix posted for that bug, I'm 
just need someone to review before I check it in.

Comment 12

[lchiang]

Tony, can you email someone on mailnewsstaff team to help review your fix?

Comment 13

[tonyr]

Checked in fix for import dialog 8/31/2000.

Comment 14

[Brian Ryner (not reading)]

Reopening: saving an attachment seems to use nsIFileSpecWithUI.  The code in
question is in nsMessenger::SaveAttachment.

Comment 15

[selmer (gone)]

Jeff Tsai, is the reopen problem yours then?  Is the problem bad enough for 
users to be a P2?  If not, this should already be a minus bug...

Comment 16

[jefft]

nsbeta3- would be fine.

Comment 17

[Stuart Parmenter]

if this bug is not fixed, we will have 2 different file pickers, which will
confuse the users a great deal, espically on linux.  the old file picker does
not behave properly in many cases, nor does it support things like filtering on
linux or mac.  I think that is very important and needs to be fixed.

Comment 18

[jefft]

Okay, let me take this bug and change priority to P2 for the time being ....

Comment 19

[jefft]

Accepting

Comment 20

[selmer (gone)]

Confirming the nsbeta3+, P2 on behalf of mailtriage.  Please clear the status
whiteboard when a bug is reopened.

Comment 21

[jefft]

Attachment: a fix

Comment 22

[jefft]

Fix checked in. r=alecf

Comment 23

[jefft]

Did a grep on all mailnews code and found out that there is an reference of
nsIFileSpecWithUI in addrbook\src\nsAddressBook.[h,cpp]. Reopenning the bug....

Comment 24

[jefft]

Reassign to the owner of the code ....

Comment 25

[chuang]

The nsAddressBook.cpp/ImportAdddressBook() which contains nsIFileSpecWithUI is 
not used in the application(mozilla or commercial) at all.  It is used before 
we have the import tool/wizard.  I keep it for debugging use (good for debugging 
migrating address book).  If it really need to be removed, please rtm++ it.  

Comment 26

[selmer (gone)]

No, don't even nominate this, I won't take it.

Comment 27

[Stuart Parmenter]

It does need to be removed so that nsIFileSpecWithUI can be removed from cvs to
reduce bloat, etc... so if this isn't fixed, we're gonna be having a red tree
for a while...

Comment 28

[Stuart Parmenter]

removing this code (or just if0'ing it out since its debug code) should be
pretty trivial.

Comment 29

[selmer (gone)]

Candice, my impression was that you were saying we don't even build the
offending code.  Is that not true then?  I could probably get approval to ifdef
it away if that's what we need to do.

Comment 30

[chuang]

I just replaced nsIFileSpecWithUI with nsIFilePicker since I'm working in the 
same file.  It didn't take me long to convert it.  So I have fix in my tree.  I 
can attach a patch but it will have fix for my other bug.

Comment 31

[lchiang]

nsbeta3 has passed.  Need to change the nsbeta3+ to a nsbeta3-.

Comment 32

[scottputterman]

removing mail2 keyword.

Comment 33

[chuang]

Attachment: Patch file for mailnews/addrbook

Comment 34

[chuang]

Attachment: Corrected Patch for mailnews/addrbook

Comment 35

[(not reading, please use [e-mail] instead)]

r=sspitzer on the current patch.  let's get rid of nsIFileSpecWithUI ASAP.

there are going to be i18n problems with this code.  nsILocalFile and the file
picker are i18n safe, but getting the path (as a C string) and setting a
nsFileSpec with it is going to do bad things if the user picks a file with
japanese characters.

adding momoi to the cc list.

there should already be a bug on "import not working with japanese file names".

I'll go try to find it.

(also, adding patch keyword.)

Comment 36

[(not reading, please use [e-mail] instead)]

I think #58109 is the import i18n bug.

Comment 37

[chuang]

Just to be clear on this patch,  the function using nsIFilePicker in 
nsAddressBook.cpp is used for debugging purpose only.  The fix for bug 58109 
will be in mailnews/import.

Comment 38

[(not reading, please use [e-mail] instead)]

sorry, my mistake.

r=sspitzer on the patch.  let's get rid of nsIFileSpecWithUI.

fyi, eventuall nsFileSpec and nsIFileSpec are going to go away too.

Comment 39

[chuang]

Attachment: Patch using diff -u

Comment 40

[Scott MacGregor]

sr=mscott

Comment 41

[chuang]

forget to mark fixed.

Comment 42

[[e-mail] (gone - use [e-mail] instead)]

http://lxr.mozilla.org/seamonkey/search?string=nsIFileSpecWithUI+ comes up 
empty (for addressbook, that is).

verified fixed.