Closed Bug 953912 Opened 10 years ago Closed 10 years ago

JS Logger line breaks don't play well on Windows

Categories

(Chat Core :: General, defect)

x86
Windows XP
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: clokep, Assigned: clokep)

Details

Attachments

(3 files)

*** Original post on bio 473 at 2010-08-12 17:30:00 UTC ***

*** Due to BzAPI limitations, the initial description is in comment 1 ***
*** Original post on bio 473 as attmnt 330 at 2010-08-12 17:30:00 UTC ***

My log files from before the JS logger were implemented work nicely in Windows (i.e. I can open them in Notepad and the hard line breaks work), now after the JS logger was implemented, everything appears as one line (i.e. Windows doesn't recognize the line breaks).

I've attached samples.

I'm not sure whether this is kind of "we don't really care since we're replacing the logs anyway" or if its a simple fix or what. I'll take a look at the JS logger code at some point and see what's in there.
Attached file Instantbird JS logger
*** Original post on bio 473 as attmnt 331 at 2010-08-12 17:30:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
*** Original post on bio 473 at 2010-08-20 18:18:31 UTC ***

I'm not sure what is better: using a single newline-character for all systems or the OS specific one.

I guess this problem will be obsolete once we have a proper log file format though.. (I think I've been saying this ways too often lately).
*** Original post on bio 473 at 2010-08-20 19:24:21 UTC ***

(In reply to comment #2)
> I'm not sure what is better: using a single newline-character for all systems
> or the OS specific one.
I'm not sure of a good reason either.

> I guess this problem will be obsolete once we have a proper log file format
> though.. (I think I've been saying this ways too often lately).
Seems a lot depends on it. Would this be blocked by that or depend on that or anything?
*** Original post on bio 473 at 2010-12-16 09:19:05 UTC ***

We had a similar issue for the copy/paste code in imThemes.jsm. The fix was https://hg.instantbird.org/instantbird/rev/ac268eda1972

You can take a similar approach here if you want, if you think it's worth fixing.
I also think the real fix would be to have a real logging system that we like, but as we probably won't get this for 0.3 and this bug may be perceived by Windows users as a regression from 0.2 which used libpurple for the logs, it may be worth fixing.
Attached patch Patch v1Splinter Review
*** Original post on bio 473 as attmnt 428 at 2010-12-16 15:11:00 UTC ***

Uses the idea from Florian's comment. This is untested, but trivial.
Attachment #8352171 - Flags: review?(florian)
Assignee: nobody → clokep
Status: NEW → ASSIGNED
Comment on attachment 8352171 [details] [diff] [review]
Patch v1

*** Original change on bio 473 attmnt 428 at 2010-12-16 19:12:53 UTC ***

We need this additionnal change in the makefile for this to work:

diff --git a/instantbird/components/Makefile.in b/instantbird/components/Makefile.in
--- a/instantbird/components/Makefile.in
+++ b/instantbird/components/Makefile.in
@@ -43,16 +43,18 @@ include $(DEPTH)/config/autoconf.mk
 
 MODULE		= instantbird
 
 XPIDLSRCS	= ibILogger.idl
 
 EXTRA_COMPONENTS = \
 	smileProtocolHandler.js smileProtocolHandler.manifest \
 	autoLoginHandler.js autoLoginHandler.manifest \
-	logger.js logger.manifest \
+	logger.manifest \
 	$(NULL)
 
+EXTRA_PP_COMPONENTS = logger.js
+
 ifeq (,$(filter mac cocoa,$(MOZ_WIDGET_TOOLKIT)))
 EXTRA_COMPONENTS += profileMigrator.js profileMigrator.manifest
 endif
 
 include $(topsrcdir)/config/rules.mk
Attachment #8352171 - Flags: review?(florian) → review+
*** Original post on bio 473 at 2010-12-16 19:21:39 UTC ***

Checked in as http://hg.instantbird.org/instantbird/rev/6a600b8a32c9
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 0.3a1
*** Original post on bio 473 at 2010-12-26 17:14:44 UTC ***

Verified since we have win32 builds again. Build: 20101226041604.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.