290914 - [FIXr]Implement nsBinaryOutputStream::WriteUtf8Z

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P2)

Description

[Boris Zbarsky [:bzbarsky]]

It seems to be well-documented and not very implemented.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

Comment 2

[Darin Fisher]

Comment on attachment 181113 [details] [diff] [review]
Patch

r+sr=darin

looks good.  we need to freeze this interface too :-)

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

OK. how does this implementation match the interface which says:

 79      * Write a NUL-terminated UTF8-encoded string to a binary stream, produced
 80      * from a NUL-terminated 16-bit PRUnichar* string argument.
 81      */
 82     void writeUtf8Z(in wstring aString);

It does not look like the written string is actually nul-terminated, since
str.Length() does not include the nul.

Additionally, this implementation differs from the other string-writing ones, in
that it does not write the length of the string to the stream. Of course, the
interface does not talk about that at all...

As it is, it seems to me like this implementation is inconsistent both with
respect to the interface and the other implementations.

Comment 4

[Darin Fisher]

hmm, yeah.. comparing the API specs for WriteStringZ, WriteWStringZ, and
WriteUtf8Z, I'd have to agree with what you are saying.  It seems to me that
WriteUtf8Z should just call:

  WriteStringZ(NS_ConvertUTF16toUTF8(input).get());

That would seem to "Write a NUL-terminated [XXX] string to a binary stream" in a
consistent manner.

We should fixup the documentation to be more clear.  We should also document the
endianness of WriteWStringZ.  These details are important if people are going to
use these interfaces to manipulate binary data.

Comment 5

[Boris Zbarsky [:bzbarsky]]

>  WriteStringZ(NS_ConvertUTF16toUTF8(input).get());

Yeah, you're right....

Note that once this has been done, there's no way to read the string back out of
the stream as Unicode, btw -- you have to readCString() and then convert from
UTF8 to UTF16 yourself....

> We should also document the endianness of WriteWStringZ. 

The whole interface is documented as writing in big-endian for all types.

Note that writing a "string" doesn't just write a string; this interface is not
designed for generally manipulating binary data but for being used with
nsIBinaryInputStream....

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Updated patch

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 181188 [details] [diff] [review]
Updated patch

Requesting 1.8b2 approval, since this is sorta an api change -- we implement
this method now.  ;)

Comment 8

[Asa Dotzler [:asa]]

Comment on attachment 181188 [details] [diff] [review]
Updated patch

a=asa

Comment 9

[Boris Zbarsky [:bzbarsky]]

Fixed