unicode utilities for mozmill and jsbridge

RESOLVED DUPLICATE of bug 506760

Status

Testing Graveyard
Mozmill
--
enhancement
RESOLVED DUPLICATE of bug 506760
7 years ago
a year ago

People

(Reporter: Jeff Hammel, Unassigned)

Tracking

Details

Attachments

(2 attachments)

(Reporter)

Description

7 years ago
Created attachment 458824 [details] [diff] [review]
file utilities

adriank has written some nice utility functions for mozmill and jsbridge to help deal with unicode.  they should be reviewed and included at some point
(Reporter)

Comment 1

7 years ago
Created attachment 458825 [details] [diff] [review]
jsbridge server utilities for unicode

here's the other one
(Reporter)

Comment 2

7 years ago
confirmed;  neither of these are applied
Comment on attachment 458824 [details] [diff] [review]
file utilities

>+    var file = Components.classes["@mozilla.org/file/local;1"]
>+            .createInstance(Components.interfaces.nsILocalFile);

To follow the code convention the leading dot of the 2nd line should be placed at the end of the first line and we don't indent with classes but Components. Don't we have Cc, and Ci defined here?

>+    var uri = ios.newFileURI(unicodeFile).spec;

this is not an URI but an URL. Would be good to make that clear here.

>+  var foStream = Components.classes["@mozilla.org/network/file-output-stream;1"]
>+                 .createInstance(Components.interfaces.nsIFileOutputStream);

snip

>+  var converter = Components.classes["@mozilla.org/intl/scriptableunicodeconverter"]
>+                         .createInstance(Components.interfaces.nsIScriptableUnicodeConverter);

snip.
Comment on attachment 458825 [details] [diff] [review]
jsbridge server utilities for unicode

>+      this.outstream = Cc['@mozilla.org/intl/converter-output-stream;1']
>+                    .createInstance(Ci.nsIConverterOutputStream);
>+      this.outstream.init(this.outputstream, 'UTF-8', 1024,
>+                    Ci.nsIConverterOutputStream.DEFAULT_REPLACEMENT_CHARACTER);
>       this.stream = transport.openInputStream(0, 0, 0);
>       this.instream = Cc['@mozilla.org/intl/converter-input-stream;1']
>           .createInstance(Ci.nsIConverterInputStream);

Please correct the indentation here too.
(Reporter)

Comment 5

7 years ago
Looking over these patches more closely, I'm not at all sure that this fixes or helps with anything.  While some of the utilities may be useful (although I'm not sure we need it now, I'm not 100% sure what problem they solve).

The JS bridge patch may be necessary.  That looks like something possibly important.  However, I'd like for the original author to comment on what it is supposed to do (and bonus points for documenting this behaviour in the code).

The patch to mozmill, however, I doubt provides the functionality that I assume it is supposed to provide -- namely, working around bug 559814.  It encodes a test file to a temporary unicode file and then loads this with mozIJSSubscriptLoader.  However, because of bug 377498, this won't work unless I'm mising a huge piece of the puzzle (See discussion on both of these bugs). The failing test case on bug 559814 *is* already in utf-8.  It is the subscript loader which insists on interpreting as utf-8.  Again, see both bugs for discussion.

To summarize:
 * the jsbridge patch looks important;  however, I'd really like the author (harth?) to comment on what problem this solves and if we need it

 * while the mozmill patch introduces some useful utilities, I don't think it actually solves any problems.  if our tests were in latin-1, there would be some utility in converting them to utf-8.  however this still wouldn't solve bug 559814 because of bug 377498 and I don't think (though I could be wrong) it solves any problems we have
(In reply to comment #0)
> adriank has written some nice utility functions for mozmill and jsbridge to
> help deal with unicode.  they should be reviewed and included at some point

a small correction: the credits and copyrights go to Heather and Clint.
I just pulled that patches from Heathers repository ;)
(Reporter)

Comment 7

7 years ago
After conversations on IRC, it sounds like these patches are part of bug 506760. Moving the patches there and marking this as a duplicate.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 506760

Updated

7 years ago
Whiteboard: [mozmill-1.4.2?] → [mozmill-1.4.2-]
Whiteboard: [mozmill-1.4.2-]
(Assignee)

Updated

a year ago
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.