351920 - Remove uses of nsSpecialSystemDirectory from mailnews

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Mark Banner (:standard8)]

nsSpecialSystemDirectory is obsolete (its in xpcom/obsolote ;-) ), there's not too many uses in mailnews so it should be fairly easy to remove. Additionally there's only one or two uses other than mailnews in the entire mozilla tree so we'll easily be able to remove it completely.

Comment 1

[Mark Banner (:standard8)]

Attachment: The fix v1

Replaces the uses of nsSpecialSystemDirectory with the new versions.

David - I'm going to run with this on linux for a while, any chance you could try a windows build?

Comment 2

[David :Bienvenu]

this looks like a candidate for a helper function or two - as it is, your patch expands a fair bit of code inline, from what i can tell. 

Comment 3

[Mark Banner (:standard8)]

Attachment: The fix v2

Same as previous, but includes helper functions where appropriate to make things a little easier.

Comment 4

[David :Bienvenu]

Comment on attachment 238287 [details] [diff] [review]
The fix v2

+  {
+    if (tmpSpec)
+      delete tmpSpec;
 
no need for the if - delete checks for null.

otherwise, this looks good. It won't be until later that I'll have a chance to compile this on windows, however.

Comment 5

[Mark Banner (:standard8)]

Comment on attachment 238287 [details] [diff] [review]
The fix v2

Requesting sr, I'll address David's comment on check-in. Hopefully he'll have tested it by then as well ;-)

Comment 6

[Scott MacGregor]

Comment on attachment 238287 [details] [diff] [review]
The fix v2

assuming you add David's comments.

Thanks for taking this on!

Comment 7

[David :Bienvenu]

Attachment: fix import files that aren't build on linux

Standard8, here are replacement diffs to get this to compile on windows.

Comment 8

[Mark Banner (:standard8)]

Attachment: One more include

Checked in the first two patches.

One #include remains in mailnews :-(

Comment 9

[David :Bienvenu]

Comment on attachment 238763 [details] [diff] [review]
One more include

you need to add #include "plstr.h" if you remove that include - sr=bienvenu if you do that.

thx for the patch!

Comment 10

[Mark Banner (:standard8)]

(In reply to comment #9)
> (From update of attachment 238763 [details] [diff] [review] [edit])
> you need to add #include "plstr.h" if you remove that include - sr=bienvenu if
> you do that.
> 
> thx for the patch!
> 
Checked in with the additional include. That's it from mailnews :-)