486259 - Non-ascii characters in log files have wrong encoding

Status: RESOLVED FIXED

(Cloud Services :: General, defect)

Description

[Igor Velkov]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b4pre) Gecko/20090331 SeaMonkey/2.0b1pre
Build Identifier: 

Non-ascii characters in weave log files writes in wrong encoding.
For example, russian text:
  2009-03-31 04:22:54	Store.Tabs.Store     DEBUG	Wrapping a tab with title LINUX.ORG.RU - Русская информация об ОС Linux

now writes into log as

2009-03-31 04:22:54	Store.Tabs.Store     DEBUG	Wrapping a tab with title LINUX.ORG.RU -  CAA:0O 8=D>@<0F8O >1 ! Linux

There should be kind of recoding.

Reproducible: Always

Comment 1

[Igor Velkov]

Attachment: add recode to utf-8

This patch work for me.
There is a strange line 
this._fos.write("", 0);
if I remove it - logging stops completely, I don't understand the reason.

"if ( true ) { " stay there for easy switch off transcoding overhead for people not need it.
Right way as I see it can add config value from list of possible encodings, or "none", or smth for avoid transcoding, but I can't implement it (don't know programming in mozilla).

Comment 2

[Dan Mills [:thunder]]

Comment on attachment 370360 [details] [diff] [review]
add recode to utf-8

I don't think this is the right way to do it.  I think the correct way is like Utils.open() does it, see here:

http://hg.mozilla.org/labs/weave/file/tip/modules/util.js#l453

note: log4moz.js doesn't depend on utils.js so that it can be used stand-alone (other projects use it, e.g. thunderbird).

Comment 3

[Igor Velkov]

Attachment: rev2

Reworked with thunder's recommendations; Thanks to myk

Comment 4

[Dan Mills [:thunder]]

Comment on attachment 372182 [details] [diff] [review]
rev2


>   doAppend: function RFApp_doAppend(message) {
>     if (message === null || message.length <= 0)
>       return;
>     try {
>       this.rotateLogs();
>-      this._fos.write(message, message.length);
>+//      this.__proto__.doAppend.call(this, message);
>+      this._fos.writeString(message);

r+ except for this commented-out line.  This use of __proto__ is incorrect - and __proto__ is bad to use in general for this purpose.  Instead, directly reference the parent object.  For example:

FileAppender.doAppend.call(this, ...);

You need to use call() to set 'this'.

If you just want to delete the line we can do that at check-in, no need to submit another patch.

Comment 5

[Igor Velkov]

I think a string 
FileAppender.prototype.doAppend.call(this, message);
will be right.
If there keep this._fos.write(message, message.length); - then 2 level logger object hierarchy concept brokes.
now it works on my profile as 

  doAppend: function RFApp_doAppend(message) {
    if (message === null || message.length <= 0)
      return;
    try {
      this.rotateLogs();
      FileAppender.prototype.doAppend.call(this, message);
    } catch(e) {
      dump("Error writing file:" + e + "\n");
    }
  },

can you accept this version?

Comment 6

[Dan Mills [:thunder]]

Fixed:

http://hg.mozilla.org/labs/weave/rev/a0d31898313a