306765 - ISO8859-1 encoding is used to upload iCal data instead of UTF-8

Status: RESOLVED FIXED

(Calendar :: Internal Components, defect)

Description

[石庭豐 (Seak, Teng-Fong)]

I'm using icalx.com to store my iCal.  I've just found that 20050831 build (and
probably quite a lot of builds before that) upload data in Latin-1 instead of
UTF-8 encoding.  For example, when I use the 'é' (e-acute) character like in
Chloé, it's displayed wrongly in icalx.com.  Downloading the raw ics file and
reading the content with a binary editor confirms this.

This bug is marked as critical because the next time Sunbird is run again, it
can no more reload the remote calendar.  Actually, that bug is more responsible
than this one, but since this is the main cause, I set this as critical.

PS: I think this bug belongs to libical. If not, please change the component.

Comment 1

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: convert to/from utf8

I just totally forgot to convert to utf8 before uploading, exporting or
importing. The patch should fix that. (One day, when i understand streams, i
should clean all this messy stream usage)

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 194704 [details] [diff] [review]
convert to/from utf8

As per discussion on IRC, this patch seems to be fixing the symptom and not the
cause.	We need to prevent the data from ever ending up in Latin-1 in the first
place.

Comment 3

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: updated patch

Updated the patch, with more comments and some symbolic constants.
The conversion isn't to convert to an utf8 string, but to a byte array. And
when downloading, to interpret the array of bytes as utf8, and convert into a
string. So this fix really is needed.

Comment 4

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 195416 [details] [diff] [review]
updated patch

>+                const segmentSize = 32768 // Must be a power of 2
For review purposes, pretend i added a ; there.

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 195416 [details] [diff] [review]
updated patch

After irc discussion, the plan is to replace the nsIStorageStream with an
nsIPipe.

Comment 6

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: use nsIPipe

Patch uses nsIPipe now.

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 195518 [details] [diff] [review]
use nsIPipe

>+                convStream.init(pipe.outputStream, 'UTF-8', 0, '?');

Passing in '?' for the default char seems like it's asking for silent data
corruption.  How about passing in 0x0000 in order to force the convStream to
throw an exception if it sees something it doesn't understand.	And do whatever
we need to propagate any such exception out to the UI.	I'd suggest the same
change for the importer.

Otherwise, looks good.

Comment 8

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: call onError

Patch calls onError when the en/decoding fails, and made the im/exporter throw.

For the onError, I added an observer that simply prompts. It needs a lot of
l10n love, but that can be done in a different bug.
(The throw from the importer is already caught, and prompts about it)

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 195781 [details] [diff] [review]
call onError

I'd slightly vote for using 0x0000 as documented in the interface file rather
null for the converter stream initializations.	But whatever you think is best.
 r=dmose with or without that change.

Comment 10

[Michiel van Leeuwen (email: mvl+moz@)]

Patch checked in

Comment 11

[Christian :Biesinger (don't email me, ping me on IRC)]

+                // This converter is needed to convert the javascript unicode 
+                // string to an array of bytes.

nsIScriptableUnicodeConverter.convertToInputStream?
http://lxr.mozilla.org/seamonkey/source/intl/uconv/idl/nsIScriptableUConv.idl#90

+    // Convert the javascript string to an araay of bytes, using the
+    // utf8 encoder

typo "araay"

Also note... the converter in/output streams are not really services... if they
were, you couldn't have two of them at the same time!

Comment 12

[Reed Loden [:reed]]

The bugspam monkeys have been set free and are feeding on Calendar :: Internal Components. Be afraid for your sanity!

Comment 13

[石庭豐 (Seak, Teng-Fong)]

What is this???  The same comment is posted on a lot of bugs, if not all. Bugzilla being spammed by robots or what?  

Comment 14

[Matthew (lilmatt) Willis]

(In reply to comment #13)
> What is this???  The same comment is posted on a lot of bugs, if not all.
> Bugzilla being spammed by robots or what?  

Many calendar bugs in bugzilla were being moved to new components.
Putting that goofy text in the comment when moving allows you to find all messages with the unique text and delete them easily.