Closed Bug 928185 Opened 6 years ago Closed 6 years ago

nsIScriptableUnicodeConverter.convertToInputStream could use an automated test

Categories

(Core :: Internationalization, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: WeirdAl, Assigned: WeirdAl)

Details

(Keywords: testcase)

Attachments

(1 file, 3 obsolete files)

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.
I haven't actually run this patch yet, but I based it on a xpcshell run with manual testing.
Attachment #818785 - Flags: review?(smontagu)
Keywords: testcase
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.
Attachment #818785 - Flags: review?(smontagu)
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.
Attachment #818785 - Attachment is obsolete: true
Attachment #819375 - Flags: review?(smontagu)
Obvious fix:  use do_check_true(original === result) instead of do_check_eq(original, result).
Attachment #819375 - Attachment is obsolete: true
Attachment #819375 - Flags: review?(smontagu)
Attachment #819377 - Flags: review?(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?
Attachment #819377 - Flags: review?(smontagu) → review+
Attachment #832740 - Flags: review+
Keywords: checkin-needed
Attachment #819377 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/bf203d83bb87
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
You need to log in before you can comment on or make changes to this bug.