It seems to be well-documented and not very implemented.
Created attachment 181113 [details] [diff] [review] Patch
13 years ago
Priority: -- → P2
Summary: Implement nsBinaryOutputStream::WriteUtf8Z → [FIX]Implement nsBinaryOutputStream::WriteUtf8Z
Target Milestone: --- → mozilla1.8beta3
Comment on attachment 181113 [details] [diff] [review] Patch r+sr=darin looks good. we need to freeze this interface too :-)
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.
OS: Linux → All
Hardware: PC → All
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.
> 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....
Created attachment 181188 [details] [diff] [review] Updated patch
13 years ago
Attachment #181188 - Flags: review?(cbiesinger) → review+
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. ;)
Attachment #181188 - Flags: approval1.8b2?
13 years ago
Summary: [FIX]Implement nsBinaryOutputStream::WriteUtf8Z → [FIXr]Implement nsBinaryOutputStream::WriteUtf8Z
Comment on attachment 181188 [details] [diff] [review] Updated patch a=asa
Attachment #181188 - Flags: approval1.8b2? → approval1.8b2+
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Target Milestone: mozilla1.8beta3 → mozilla1.8beta2
You need to log in before you can comment on or make changes to this bug.