Closed
Bug 306765
Opened 19 years ago
Closed 19 years ago
ISO8859-1 encoding is used to upload iCal data instead of UTF-8
Categories
(Calendar :: Internal Components, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: lapsap7+mz, Assigned: mvl)
References
Details
Attachments
(1 file, 3 obsolete files)
9.75 KB,
patch
|
dmosedale
:
first-review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•19 years ago
|
||
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•19 years ago
|
||
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.
Attachment #194704 -
Flags: first-review?(dmose) → first-review-
Assignee | ||
Comment 3•19 years ago
|
||
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.
Attachment #194704 -
Attachment is obsolete: true
Attachment #195416 -
Flags: first-review?(dmose)
Assignee | ||
Comment 4•19 years ago
|
||
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•19 years ago
|
||
Comment on attachment 195416 [details] [diff] [review] updated patch After irc discussion, the plan is to replace the nsIStorageStream with an nsIPipe.
Attachment #195416 -
Flags: first-review?(dmose)
Assignee | ||
Comment 6•19 years ago
|
||
Patch uses nsIPipe now.
Attachment #195416 -
Attachment is obsolete: true
Attachment #195518 -
Flags: first-review?(dmose)
Comment 7•19 years ago
|
||
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.
Attachment #195518 -
Flags: first-review?(dmose) → first-review-
Assignee | ||
Comment 8•19 years ago
|
||
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)
Attachment #195518 -
Attachment is obsolete: true
Attachment #195781 -
Flags: first-review?(dmose)
Comment 9•19 years ago
|
||
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.
Attachment #195781 -
Flags: first-review?(dmose) → first-review+
Assignee | ||
Comment 10•19 years ago
|
||
Patch checked in
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Comment 11•19 years ago
|
||
+ // 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!
Updated•18 years ago
|
Component: libical → Internal Components
Comment 12•18 years ago
|
||
The bugspam monkeys have been set free and are feeding on Calendar :: Internal Components. Be afraid for your sanity!
QA Contact: libical → base
Reporter | ||
Comment 13•18 years ago
|
||
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•18 years ago
|
||
(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.
You need to log in
before you can comment on or make changes to this bug.
Description
•