68651 - streamConverter should use NS_LITERAL_STRING

Status: VERIFIED FIXED

(Core Graveyard :: Networking: FTP, defect)

Description

[Bradley Baetz]

While looking at the ftp changes, I noticed that its doing a whole lot of
.AssignWithConversion("String constant"), instead of using NS_LITERAL_STRING.
The above URL is one example, but there are others.

Comment 1

[Bradley Baetz]

Attachment: patch

Comment 2

[Peter "jag" Annema]

+    nsAutoString fromStr; fromStr.Assign(NS_LITERAL_STRING("text/ftp-dir-"));

nsAutoString fromStr(NS_LITERAL_STRING("text/ftp-dir-"));

is slightly more efficient.

r=jag

Is there a bug yet on localizing those lines you're touching?

Comment 3

[Bradley Baetz]

OK, new patch coming up.

> Is there a bug yet on localizing those lines you're touching?

don't know. I filed a bug (which got helpwanted and FUTUREd) on printing the
server's actual reply though. If I do that, that string won't be localised.

Comment 4

[Bradley Baetz]

Attachment: new patch

Comment 5

[Peter "jag" Annema]

r=jag

Comment 6

[Doug Turner (:dougt)]

Make sure it works on windows.  I once had problems compiling on windows with
the NS_LITERAL_STRING.  I had to use a .get().

Ensure that and sr=dougt.


can I get you to look at the ftp stream converter :-)

Comment 7

[Bradley Baetz]

I'll look at the stream converter, and get a windows person to look at this
patch. I'm building with -fshort-wchar, so I should be getting the same types of
strings that the windows people are.

One thing I noticed is that the streamconverter handles a whole lot of ftp
server types which aren't handled by the ftp protocol handler. Am I missing
something?

Comment 8

[Bradley Baetz]

Oh, and reassigning so that I can find it when I get arround to applying for CVS
write access.

Comment 9

[Doug Turner (:dougt)]

The ftp implemention (protocol and converter) were based on the classic code
which "supported" more ftp servers then what mozilla does.  I bet this mismatch
is because we did not removed some of the server type from the converter.  No
harm, i guess.

Comment 10

[Bradley Baetz]

But you added VMS into the stream converter with the big necko branch landing,
but didn't touch the protocol handler. It doesn't matter, I suppose.

Comment 11

[Bradley Baetz]

walk84 compiled it on windows with no problems.

Comment 12

[Bradley Baetz]

jag checked the fix in for me. I'll leave this open for the stream converter
changes.

Oh, and ignore what I was talking about wrt VMS. Don't know where I got that
from.

Comment 13

[Bradley Baetz]

The streamConverter stuff would benefit from the new strings, I think. I'll fix
this when that lands.

Comment 14

[Bradley Baetz]

Morphing.

Comment 15

[Bradley Baetz]

Done as part of bug 78148

Comment 16

[Bradley Baetz]

pushing off - this is too late for 0.9.1

Comment 17

[Bradley Baetz (:bbaetz)]

Mass change: Bug 78148 has been checked in, and so it and these dependancies
should have been fixed. If you disagree, please reopen.

Note that the xul view is still enabled for file because of i18n problems. The
bug to fix this is bug 102812.

Comment 18

[benc]

jag or dougt: since this is purely a code change (no external black-box test),
would one of you mark this verified?

Comment 19

[benc]

VERIFIED: 
Poked around in the URL provided (in LXR) and couldn't find any of the offending
code.