928185 - nsIScriptableUnicodeConverter.convertToInputStream could use an automated test

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[Alex Vincent [:WeirdAl]]

This is a blatant excuse for me to try learning about character encodings.  Especially since I am maintaining a XPCOM streams guide on devmo, and don't have a good understanding of them.

I was feeding a SAXXMLReader with a StringInputStream... only I didn't realize that stream requires 8-bit characters, and I was initializing the stream with regular JavaScript strings (which use 16-bit characters).

var stream = new StringInputStream(data, data.length); // bad when data is a JS string

So I realized I should've been building my input stream using a conversion from JS strings.  In investigating how SAXXMLParser does it (SAXXMLParser::ParseFromString), I eventually discovered the nsIScriptableUnicodeConverter interface.

Comment 1

[Alex Vincent [:WeirdAl]]

Attachment: Test converting from JS string to UTF-8 input stream and back

I haven't actually run this patch yet, but I based it on a xpcshell run with manual testing.

Comment 2

[Alex Vincent [:WeirdAl]]

Comment on attachment 818785 [details] [diff] [review]
Test converting from JS string to UTF-8 input stream and back

... and if I had tested, it would've failed with a rather obvious error.  D'oh!

New patch coming with fix as soon as possible.

Comment 3

[Alex Vincent [:WeirdAl]]

Attachment: Test converting from JS string to UTF-8 input stream and back

I need some help.  This patch passes on Windows 7, but fails on Fedora Linux when run as ./mach xpcshell-test:

UnicodeDecodeError: 'ascii' codec can't decode byte 0xfd in position 181: ordinal not in range(128)

But when I run it through ./mach xpcshell-test --interactive, it passes.  The UnicodeDecodeError is apparently coming from Python, not from JS code.

Comment 4

[Alex Vincent [:WeirdAl]]

Attachment: Test converting from JS string to UTF-8 input stream and back

Obvious fix:  use do_check_true(original === result) instead of do_check_eq(original, result).

Comment 5

[Simon Montagu :smontagu]

Comment on attachment 819377 [details] [diff] [review]
Test converting from JS string to UTF-8 input stream and back

Review of attachment 819377 [details] [diff] [review]:
-----------------------------------------------------------------

I was sure I had commented on this already :-S

::: intl/uconv/tests/unit/test_input_stream.js
@@ +20,5 @@
> +    /* Python bug: UnicodeDecodeError: 'ascii' codec can't decode byte 0xfd in position 181: ordinal not in range(128)
> +    do_check_eq(original, result);
> +    */
> +    do_check_true(original === result);
> +}

This isn't great for finding what's gone wrong if the test fails. How about |do_check_eq(escape(original), escape(result))| like I do in a lot of the other tests here?

Comment 6

[Alex Vincent [:WeirdAl]]

Attachment: updated per review comments

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bf203d83bb87

Comment 8

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/bf203d83bb87