Eliminate nsFileSpec and nsIFileSpec (references) from MailNews

RESOLVED FIXED in Thunderbird 3.1a1

Status

MailNews Core
Backend
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: sgautherie, Assigned: sgautherie)

Tracking

Trunk
Thunderbird 3.1a1
Dependency tree / graph
Bug Flags:
wanted-thunderbird3 +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(10 attachments, 1 obsolete attachment)

7.13 KB, patch
Bienvenu
: review+
Bienvenu
: superreview+
Details | Diff | Splinter Review
1.67 KB, patch
Bienvenu
: review+
Bienvenu
: superreview+
Details | Diff | Splinter Review
7.78 KB, patch
neil@parkwaycc.co.uk
: review+
neil@parkwaycc.co.uk
: superreview+
Details | Diff | Splinter Review
2.99 KB, patch
Bienvenu
: review+
Bienvenu
: superreview+
Details | Diff | Splinter Review
1.87 KB, patch
Bienvenu
: review+
Bienvenu
: superreview+
Details | Diff | Splinter Review
1.19 KB, patch
Bienvenu
: review+
Bienvenu
: superreview+
Details | Diff | Splinter Review
3.50 KB, patch
Bienvenu
: review+
Bienvenu
: superreview+
Details | Diff | Splinter Review
6.31 KB, patch
Bienvenu
: review+
Bienvenu
: superreview+
Details | Diff | Splinter Review
1.80 KB, patch
Bienvenu
: review+
Bienvenu
: superreview+
Details | Diff | Splinter Review
2.16 KB, patch
Bienvenu
: review+
Bienvenu
: superreview+
Details | Diff | Splinter Review
Comment hidden (empty)
(Assignee)

Updated

10 years ago
Depends on: 33451
(Assignee)

Updated

10 years ago
Depends on: 355008
(Assignee)

Comment 1

10 years ago
Created attachment 342938 [details] [diff] [review]
(Cv1) <amUtils.js>

Update quite old code.
Assignee: nobody → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #342938 - Flags: superreview?(neil)
Attachment #342938 - Flags: review?(neil)
(Assignee)

Comment 2

10 years ago
Created attachment 342939 [details] [diff] [review]
(Av1) Bug 33451 followup
[Checkin: Comment 4]
Attachment #342939 - Flags: superreview?(bienvenu)
Attachment #342939 - Flags: review?(bienvenu)
(Assignee)

Comment 3

10 years ago
Created attachment 342940 [details] [diff] [review]
(Bv1) Bug 355008 followup
[Checkin: Comment 5]
Attachment #342940 - Flags: superreview?(bienvenu)
Attachment #342940 - Flags: review?(bienvenu)

Updated

10 years ago
Attachment #342939 - Flags: superreview?(bienvenu)
Attachment #342939 - Flags: superreview+
Attachment #342939 - Flags: review?(bienvenu)
Attachment #342939 - Flags: review+

Updated

10 years ago
Attachment #342940 - Flags: superreview?(bienvenu)
Attachment #342940 - Flags: superreview+
Attachment #342940 - Flags: review?(bienvenu)
Attachment #342940 - Flags: review+
(Assignee)

Comment 4

10 years ago
Comment on attachment 342939 [details] [diff] [review]
(Av1) Bug 33451 followup
[Checkin: Comment 4]

http://hg.mozilla.org/comm-central/rev/cbb98a8d6e8d
Attachment #342939 - Attachment description: (Av1) Bug 33451 followup → (Av1) Bug 33451 followup [Checkin: Comment 4]
(Assignee)

Comment 5

10 years ago
Comment on attachment 342940 [details] [diff] [review]
(Bv1) Bug 355008 followup
[Checkin: Comment 5]

http://hg.mozilla.org/comm-central/rev/c5f2947a86f1
Attachment #342940 - Attachment description: (Bv1) Bug 355008 followup → (Bv1) Bug 355008 followup [Checkin: Comment 5]

Comment 6

10 years ago
Comment on attachment 342938 [details] [diff] [review]
(Cv1) <amUtils.js>

>   var ret = fp.show();
> 
>-  if (ret == nsIFilePicker.returnOK) 
>+  if (ret == nsIFilePicker.returnOK)
Nit: could inline ret

>+      var currentServer = allServers.GetElementAt(i)
>+                                    .QueryInterface(Components.interfaces.nsIMsgIncomingServer);
Nit: should use QueryElementAt

>+          currentServer.localPath.nativePath == folderPath)
This should use .localPath.equals(fp.file)
(It doesn't work as-is, because nativePath isn't scriptable on nsIFile)
Attachment #342938 - Flags: superreview?(neil)
Attachment #342938 - Flags: superreview-
Attachment #342938 - Flags: review?(neil)
(Assignee)

Comment 7

10 years ago
Created attachment 343326 [details] [diff] [review]
(Cv2) <amUtils.js>
[Checkin: Comment 9]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081014 SeaMonkey/2.0a2pre] (nightly) (W2Ksp4)

Cv1, with comment 6 suggestion(s),
plus a more detailed error message.
Attachment #342938 - Attachment is obsolete: true
Attachment #343326 - Flags: superreview?(neil)
Attachment #343326 - Flags: review?(neil)
(Assignee)

Comment 8

10 years ago
David, could you fix
{
/mailnews/import/outlook/src/nsOutlookMail.cpp
    * line 714 -- nsCOMPtr<nsIFileSpec> pSpec;
}
which is a leftover from bug 33451 ?
(Update the code or maybe remove this commented out function !?)
Thanks.

Updated

10 years ago
Attachment #343326 - Flags: superreview?(neil)
Attachment #343326 - Flags: superreview+
Attachment #343326 - Flags: review?(neil)
Attachment #343326 - Flags: review+
(Assignee)

Updated

10 years ago
Attachment #343326 - Attachment description: (Cv2) <amUtils.js> → (Cv2) <amUtils.js> [Checkin: Comment 9]
(Assignee)

Comment 10

10 years ago
Created attachment 344214 [details] [diff] [review]
(Dv1) Bug 33451 <nsMsgMailNewsUrl.cpp> followup ++
[Checkin: Comment 12]

(compiled, not run, but quite obvious)
Attachment #344214 - Flags: superreview?(bienvenu)
Attachment #344214 - Flags: review?(bienvenu)

Updated

10 years ago
Attachment #344214 - Flags: superreview?(bienvenu)
Attachment #344214 - Flags: superreview+
Attachment #344214 - Flags: review?(bienvenu)
Attachment #344214 - Flags: review+

Comment 11

10 years ago
Comment on attachment 344214 [details] [diff] [review]
(Dv1) Bug 33451 <nsMsgMailNewsUrl.cpp> followup ++
[Checkin: Comment 12]

r/sr=me, if you move the decl of nsresult down to where it's used:

 nsresult nsMsgSaveAsListener::SetupMsgWriteStream(nsIFile *aFile, PRBool addDummyEnvelope)
 {
-  nsresult rv = NS_ERROR_FAILURE;
+  nsresult rv;
(Assignee)

Updated

10 years ago
Attachment #344214 - Attachment description: (Dv1) Bug 33451 <nsMsgMailNewsUrl.cpp> followup ++ → (Dv1) Bug 33451 <nsMsgMailNewsUrl.cpp> followup ++ [Checkin: Comment 12]
(Assignee)

Comment 12

10 years ago
Comment on attachment 344214 [details] [diff] [review]
(Dv1) Bug 33451 <nsMsgMailNewsUrl.cpp> followup ++
[Checkin: Comment 12]

http://hg.mozilla.org/comm-central/rev/4992b24a9b49
(Assignee)

Comment 14

10 years ago
Created attachment 344326 [details] [diff] [review]
(Ev1) Bug 33451 <nsImapMailFolder.cpp> followup
[Checkin: Comment 15]
Attachment #344326 - Flags: superreview?(bienvenu)
Attachment #344326 - Flags: review?(bienvenu)

Updated

10 years ago
Attachment #344326 - Flags: superreview?(bienvenu)
Attachment #344326 - Flags: superreview+
Attachment #344326 - Flags: review?(bienvenu)
Attachment #344326 - Flags: review+
(Assignee)

Comment 15

10 years ago
Comment on attachment 344326 [details] [diff] [review]
(Ev1) Bug 33451 <nsImapMailFolder.cpp> followup
[Checkin: Comment 15]

http://hg.mozilla.org/comm-central/rev/13727f26b4a9
Attachment #344326 - Attachment description: (Ev1) Bug 33451 <nsImapMailFolder.cpp> followup → (Ev1) Bug 33451 <nsImapMailFolder.cpp> followup [Checkin: Comment 15]
(Assignee)

Comment 16

10 years ago
Created attachment 344419 [details] [diff] [review]
(Fv1) Bug 98391 then bug 33451 <nsLocalMailFolder.cpp> followup
[Checkin: Comment 17]

I don't know if using moveTo() would be "better" or not. (as a followup)

See
http://mxr.mozilla.org/comm-central/source/mozilla/xpcom/io/nsIFile.idl?mark=195-196#163
Attachment #344419 - Flags: superreview?(bienvenu)
Attachment #344419 - Flags: review?(bienvenu)

Updated

10 years ago
Attachment #344419 - Flags: superreview?(bienvenu)
Attachment #344419 - Flags: superreview+
Attachment #344419 - Flags: review?(bienvenu)
Attachment #344419 - Flags: review+
(Assignee)

Comment 17

10 years ago
Comment on attachment 344419 [details] [diff] [review]
(Fv1) Bug 98391 then bug 33451 <nsLocalMailFolder.cpp> followup
[Checkin: Comment 17]

http://hg.mozilla.org/comm-central/rev/9c1554e52ae8
Attachment #344419 - Attachment description: (Fv1) Bug 98391 then bug 33451 <nsLocalMailFolder.cpp> followup → (Fv1) Bug 98391 then bug 33451 <nsLocalMailFolder.cpp> followup [Checkin: Comment 17]
(Assignee)

Comment 18

10 years ago
David, could you fix
{
/mailnews/local/src/nsMailboxUrl.cpp
    * line 174 -- // mURI isn't set due to nsFileSpec/nsIFile fun.
}
which seems to be a leftover from bug 33451 ?
Thanks.

See
http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/mailnews/local/src/nsMailboxUrl.h&mark=1.44
http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/mailnews/local/src/nsMailboxUrl.cpp&mark=1.102#1.106
when |m_filePath| and |m_messageFile| changed from nsFileSpec to nsILocalFile.
(Assignee)

Comment 19

10 years ago
David, could you fix
{
/mailnews/import/eudora/src/nsEudoraMailbox.cpp
    * line 447 -- // to reopen it because the nsIFileSpec implementation will open
}
which seems to be a "missed update" from bug 368347 3rd patch ?
Thanks.

See
http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/mailnews/import/eudora/src/nsEudoraMailbox.cpp&mark=1.31
when this function (with |nsIOutputStream *pDst|) and comment were added.
Depends on: 368347
Flags: wanted-thunderbird3?
Keywords: helpwanted
Target Milestone: --- → mozilla1.9.1b2
(Assignee)

Comment 20

10 years ago
Created attachment 345911 [details] [diff] [review]
(Gv1) Bug 33451 <nsMsgDatabase.*> followup
[Checkin: Comment 21]

Its only usage was removed by
http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/mailnews/db/msgdb/src/nsMsgDatabase.cpp&mark=1.377#1.378
Attachment #345911 - Flags: superreview?(bienvenu)
Attachment #345911 - Flags: review?(bienvenu)
(Assignee)

Updated

10 years ago
Whiteboard: [ToDo: comments 8, 18 and 19]

Updated

10 years ago
Attachment #345911 - Flags: superreview?(bienvenu)
Attachment #345911 - Flags: superreview+
Attachment #345911 - Flags: review?(bienvenu)
Attachment #345911 - Flags: review+
(Assignee)

Comment 21

10 years ago
Comment on attachment 345911 [details] [diff] [review]
(Gv1) Bug 33451 <nsMsgDatabase.*> followup
[Checkin: Comment 21]

http://hg.mozilla.org/comm-central/rev/2fd2d404bcd3
Attachment #345911 - Attachment description: (Gv1) Bug 33451 <nsMsgDatabase.*> followup → (Gv1) Bug 33451 <nsMsgDatabase.*> followup [Checkin: Comment 21]

Comment 22

10 years ago
wanted‑thunderbird3+
Serge, are you working on these? Only tree references to go now!

The commented out outlook function should just be removed. For eudora, the matching line and a couple of uncommented lines after it. For the last comment ref, I think it should just be "isn't set due to nsIFile fun"
Flags: wanted-thunderbird3? → wanted-thunderbird3+
(Assignee)

Comment 23

9 years ago
Created attachment 406385 [details] [diff] [review]
(Hv1) Bug 33451 <nsOutlookMail.cpp> followup
[Checkin: Comment 29]

Per comment 8 and comment 22.
Attachment #406385 - Flags: superreview?(bienvenu)
Attachment #406385 - Flags: review?(bienvenu)
Attachment #406385 - Flags: approval-thunderbird3?
Comment on attachment 406385 [details] [diff] [review]
(Hv1) Bug 33451 <nsOutlookMail.cpp> followup
[Checkin: Comment 29]

Serge, please don't request approval until you have a reviewed patch - this way we can track requests better and spend less time working out what has/hasn't been reviewed.

Please also include risk statements when requesting approval as to what the patch does and could affect if it is wrong - even if brief.
Attachment #406385 - Flags: approval-thunderbird3?

Comment 25

9 years ago
Comment on attachment 406385 [details] [diff] [review]
(Hv1) Bug 33451 <nsOutlookMail.cpp> followup
[Checkin: Comment 29]

I see no risk but no reward either for landing this in 3.0 since it's merely removing commented out code.
Attachment #406385 - Flags: superreview?(bienvenu)
Attachment #406385 - Flags: superreview+
Attachment #406385 - Flags: review?(bienvenu)
Attachment #406385 - Flags: review+
(Assignee)

Comment 26

9 years ago
Comment on attachment 406385 [details] [diff] [review]
(Hv1) Bug 33451 <nsOutlookMail.cpp> followup
[Checkin: Comment 29]


(In reply to comment #25)
> I see no risk but no reward either for landing this in 3.0 since it's merely
> removing commented out code.

100% agreed, but it would let me(/us) move forward.
Attachment #406385 - Flags: approval-thunderbird3?
(Assignee)

Comment 27

9 years ago
Created attachment 406634 [details] [diff] [review]
(Iv1) Bug 33451 <nsMailboxUrl.cpp> followup ++
[Checkin: Comment 30]

Per comment 18 and comment 22.
(Note that I have no idea what the comment should be or whether it even still applies.)
Attachment #406634 - Flags: superreview?(bienvenu)
Attachment #406634 - Flags: review?(bienvenu)
Comment on attachment 406385 [details] [diff] [review]
(Hv1) Bug 33451 <nsOutlookMail.cpp> followup
[Checkin: Comment 29]

a=Standard8 as this is comment-only removals, and only if this lands before we branch (once we branch it will be very unlikely that tidy up patches will land, but as this is comment only, then I see low risk).
Attachment #406385 - Flags: approval-thunderbird3? → approval-thunderbird3+
(Assignee)

Comment 29

9 years ago
Comment on attachment 406385 [details] [diff] [review]
(Hv1) Bug 33451 <nsOutlookMail.cpp> followup
[Checkin: Comment 29]


http://hg.mozilla.org/comm-central/rev/25fe4f5e7702
Attachment #406385 - Attachment description: (Hv1) Bug 33451 <nsOutlookMail.cpp> followup → (Hv1) Bug 33451 <nsOutlookMail.cpp> followup [Checkin: Comment 29]

Updated

9 years ago
Attachment #406634 - Flags: superreview?(bienvenu)
Attachment #406634 - Flags: superreview+
Attachment #406634 - Flags: review?(bienvenu)
Attachment #406634 - Flags: review+
(Assignee)

Comment 30

9 years ago
Comment on attachment 406634 [details] [diff] [review]
(Iv1) Bug 33451 <nsMailboxUrl.cpp> followup ++
[Checkin: Comment 30]


http://hg.mozilla.org/comm-central/rev/02601e5f7bb2
Attachment #406634 - Attachment description: (Iv1) Bug 33451 <nsMailboxUrl.cpp> followup ++ → (Iv1) Bug 33451 <nsMailboxUrl.cpp> followup ++ [Checkin: Comment 30]
(Assignee)

Comment 31

9 years ago
Created attachment 412334 [details] [diff] [review]
(Jv1) Bug 368347 <nsEudoraMailbox.cpp> followup
[Checkin: Comment 32]

Per comment 19 and comment 22.
Attachment #412334 - Flags: superreview?(bienvenu)
Attachment #412334 - Flags: review?(bienvenu)
(Assignee)

Updated

9 years ago
Keywords: helpwanted
Whiteboard: [ToDo: comments 8, 18 and 19]

Updated

9 years ago
Attachment #412334 - Flags: superreview?(bienvenu)
Attachment #412334 - Flags: superreview+
Attachment #412334 - Flags: review?(bienvenu)
Attachment #412334 - Flags: review+
(Assignee)

Comment 32

9 years ago
Comment on attachment 412334 [details] [diff] [review]
(Jv1) Bug 368347 <nsEudoraMailbox.cpp> followup
[Checkin: Comment 32]


http://hg.mozilla.org/comm-central/rev/6ea33184f759
Attachment #412334 - Attachment description: (Jv1) Bug 368347 <nsEudoraMailbox.cpp> followup → (Jv1) Bug 368347 <nsEudoraMailbox.cpp> followup [Checkin: Comment 32]
(Assignee)

Updated

9 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: mozilla1.9.1b2 → Thunderbird 3.1a1
You need to log in before you can comment on or make changes to this bug.