359083 - sunbird calendar csv-export wrong time forgetting about pm all times are am

Status: RESOLVED WONTFIX

(Calendar :: Import and Export, defect)

Description

[h.p.bernhard]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.1) Gecko/20061010 Firefox/2.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.1) Gecko/20061010 Firefox/2.0

If a calendar or events are exported do cvs the exported 
text file has only time values between 0:00 an 12:59.
all afternoon events are mapped to am 14:20 -> 2:20

Reproducible: Always

Steps to Reproduce:
1.export calender or events as csv file
2.analyse the csv file

Comment 1

[Damian Szczepanik]

confirmed - event as AM are exported the same as PM so you can't see differences in csv file

it looks like quite easy to fix

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20070112 Calendar/0.4a1

Comment 2

[Stefan Sitter [:ssitter]]

Currently only export to CSV format that matches English locale is possible. The time format in output is forced to match English format:

http://lxr.mozilla.org/mozilla/source/calendar/import-export/calOutlookCSVImportExport.js#97

    timeFormat: "%I:%M:%S %p"

%I is replaced by the hour (12-hour clock)
%p is replaced by the locale's equivalent of either AM or PM. 

But on my German Windows 2000 system there is no equivalent defined for AM or PM. As the result %p is replaced with a blank string. The reporter also seems to use a German system and Damian probably used a Polish system that has the same problem.

I was able to workaround this by going to Start Menu -> Settings -> Control Panel -> Regional and Language Options -> Time and setting the strings for AM/PM from empty "" to "AM" and "PM".

Comment 4

[:Hb]

Another workaround is to just change the definiton for the output to "timeFormat: "%H:%M:%S""

Full description at: http://www.sunbird-kalender.de/forum/viewtopic.php?p=8490#8490

Comment 5

[:Hb]

Attachment: Patch for calOutlookCSVImportExport.js

Sunbird and Lightning 0.7 can't import CSV files exported by them. 

This patch solves this bug 359083 with allowing 12 hrs am/pm and 24 hrs time format to be read from a CSV import file.

It solves bug 366921 setting the CSV export format to MM/DD/YYYY with 4 digit years and to HH:MM:SS with 24 hours.

It gives a workaround for bug 337377 by reducing the number of mandatory columns to six. 

It solves bug 405737 by now subtracting the start date from the alarm date which gives the necessary negative result.

It allows events to be successfully imported in cases were the begin time was not successfull recognized, see bug 362496 comment 10. 

It adds a German localization for importing CSV files exported from MS Outlook (Versionen 98 bis 2003 deutsch).

Comment 6

[:Hb]

Attachment: calOutlookCSVImportExport.js - Complete file for Sunbird/Lightning 0.7

Just replace C:\Program files\Mozilla Sunbird\js\calOutlookCSVImportExport.js with this file and have your CSV files imported in Sunbird 0.7.

Comment 7

[:Hb]

Attachment: calOutlookCSVImportExport.js - Complete file for Sunbird/LG 0.8

For use with the Sunbird/Lightning 0.8 nightlies only.

Comment 8

[:Hb]

Attachment: calOutlookCSVImportExport.js - Complete file for Sunbird/LG 0.8

For use with the Sunbird/Lightning 0.8 nightlies only.

Comment 9

[:Hb]

Attachment: Patch for calOutlookCSVImportExport.js

Line 329 corrected

Comment 10

[Philipp Kewisch [:Fallen][☁️📆]]

I'd say since there is much locale specific stuff in there, we should move the strings into a properties file so that localizers can properly translate. Hb, could you take care of that?

Comment 11

[Stefan Sitter [:ssitter]]

The csv file format is independent of the Sunbird/Lightning localization. It depends on the Outlook version and localization used to created the file. See Bug 301750.

Comment 12

[Philipp Kewisch [:Fallen][☁️📆]]

Yes, I am aware of this. I was thinking that since it doesn't only depend on version but also on language, we could create properties like:


outlookCSVImport.2003.fields.headTitle = "Betreff"

This of course has nothing to with sb/ltn languages, but this way we can easily support more CSV import languagess. It is indeed something for but 301750 though. I didn't look at all lines of Hb's patches, but in case the fix is to add german localization, I'd say this is a dupe of bug 301750.

Comment 13

[Martin Schröder [:mschroeder]]

Hb, don't forget to ask for review like I've told you. :)

Comment 14

[:Hb]

Attachment: calOutlookCSVImportExport.js - Complete file for Sunbird/Lightning 0.7

Just replace with this file the file calOutlookCSVImportExport.js from your current Sunbird/Lightning 0.7 installation and have your CSV files ex- and imported.

It is in C:\Program_files\Mozilla Sunbird\js\ for Sunbird or for TB in 
C:\Documents and Settings\<your username>\Application Data\
Thunderbird \Profiles\<random string>.default\extensions\ {e2fda1a4-762b-4020-b5ad-a41df1933103}\js\

Comment 15

[:Hb]

Attachment: calOutlookCSVImportExport.js - Complete file for SB/LG 0.8

For use with the Sunbird/Lightning 0.8 nightlies only.

Comment 16

[:Hb]

Attachment: Patch (Version 2) for calOutlookCSVImportExport.js

This patch solves this bug 359083 with allowing 12 hrs am/pm and 24 hrs time
format to be read from a CSV import file.

It solves bug 366921 with setting the CSV export format to MM/DD/YYYY with 4 digit years and to HH:MM:SS with 24 hours. Sunbird/TB can now import their own export files. Additionally all 2 digit years are imported as of the year 2000 and beyond.

It gives a workaround for bug 337377 by reducing the number of mandatory
columns to six. 

It solves bug 405737 by now subtracting the start date from the alarm date
which gives the necessary negative result.

It allows events to be successfully imported in cases were the time part of the starting date/time was not successfull recognized, see bug 362496 comment 10. 

It adds German and French localizations for importing CSV files exported from MS Outlook (Versionen 98 bis 2003 deutsch and french).

It makes the fallback mechanism "default indexes for a default Outlook2000 CSV file" workable through setting the variables knownIndxs and needIndxs.

It allows to localize the export format with a "const=value" change in the code.

Line endings in the export file are now CR+LF, allowing Outlook to read this files.

Not unset categories are now exported as empty strings and not as "none".

Comment 17

[:Hb]

Attachment: CSV test file from french Outlook

Comment 18

[:Hb]

This major bug depends not on bug 336175. It can be resolved solely.

Comment 19

[Bas van den Bosch]

I like fixes to the csv-system, quite often there's questions about this in the forums. A few questions:
Can exportLocale and timeFormat become settings in the advanced configuration or perhaps an l10n-thing? 
Does the locale of the import-csv get detected automatically?
Are events set to floating TZ? shouldn't these get the application-timezone?

Comment 20

[:Hb]

Attachment: Patch (Version 3) for calOutlookCSVImportExport.js

(In reply to comment #19)
> I like fixes to the csv-system, quite often there's questions about this 
> in the forums. A few questions:  Can exportLocale and timeFormat become 
> settings in  the advanced configuration or perhaps an l10n-thing? 

Some beginnings for this are made in the localization files, see
http://mxr.mozilla.org/l10n/source/de/calendar/chrome/calendar/calendar.properties#165
It is questionable if completing this will be possible for the next release.

It's more important that Sunbird can import the CSV files exported from itself. This is imperative for each version. 

> Does the locale of the import-csv get detected automatically?

Yes for the languages DE, EN, FR and NL.

> Are events set to floating TZ? shouldn't these get the application-timezone?

Yes. Version 3 now uses calendarDefaultTimezone() instead of "floating" or floating().

Comment 21

[:Hb]

Attachment: calOutlookCSVImportExport.js - Complete file for Sunbird/LG versions 0.7 and 0.8

Just replace the file calOutlookCSVImportExport.js from your current Sunbird/Lightning installations with this file and have your CSV files ex- and
imported.

It is in C:\Program_files\Mozilla Sunbird\js\ for Sunbird.
Thunderbird/Lightning has it in
C:\Documents and Settings\<your username>\Application Data\
Thunderbird \Profiles\<random string>.default\extensions\
{e2fda1a4-762b-4020-b5ad-a41df1933103}\js\

Comment 22

[:Hb]

Attachment: CSV test files, collection for DE, EN and FR and Outlook 98, 2K, XP and 2003

Contains file 299344 and five more samples togehter with an excel file to construct more samples. Zip format.

Comment 23

[:Hb]

Due to bug 336175 the export of a complete calendar still fails if it contains tasks. But it is possible to select all events and export only them.

Comment 24

[:Hb]

Attachment: calOutlookCSVImportExport.js - Complete file for Sunbird/LG versions 0.7 and 0.8

Version 4 of this patch nearly resolves Bug 336175 too. Todo items are not exported.

Comment 25

[:Hb]

Attachment: Patch (Version 4) for calOutlookCSVImportExport.js

For resolving bug 336175 too this version exports only event items. 

Todo items ares skipped in the function csv_exportToStream(...) with "if (item instanceof Components.interfaces.calIEvent) {...}"

Comment 26

[:Hb]

Attachment: FINAL patch (Version 5) for calOutlookCSVImportExport.js

Set the import range for 2-digit years to 1930 till 2029. This setting is mostly used on windows installations.

Multiple double quotes are now escaped properly on export.

This is the final patch.

Comment 27

[:Hb]

Attachment: calOutlookCSVImportExport.js - Complete file for Sunbird/Ligthning versions 0.7 and 0.8

Comment 28

[Daniel Boelzle [:dbo]]

Comment on attachment 299779 [details] [diff] [review]
FINAL patch (Version 5) for calOutlookCSVImportExport.js

Sorry Ernst, I am neither owner nor peer for import/export; moving review request to mvl.

Comment 29

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

Comment on attachment 299779 [details] [diff] [review]
FINAL patch (Version 5) for calOutlookCSVImportExport.js

>Index: calendar/import-export/calOutlookCSVImportExport.js
>@@ -171,34 +238,35 @@ function csv_importFromStream(aStream, a
>+                    case locale.headTitle:        args.titleIndex = i;       knownIndxs++; needIndxs++; break;
>-            if (knownIndxs >= 13)
>+            if (needIndxs >= 6)

That will stop the loop after the 6 required fields are found. But any other field that's not required but present, will be ignored then. That's not good.

Comment 30

[:Hb]

Attachment: CSV test case with only 10 columns

This csv file has only ten columns. The three alarm related columns are missing. All information from the six "needed" columns and from the additional columns "location" and "description" are imported.

Comment 31

[cmtalbert]

Attachment: event CSV export from outlook 07

Comment 32

[cmtalbert]

Attachment: Tasks CSV export from outlook 2007

Comment 33

[:Hb]

Comment on attachment 299779 [details] [diff] [review]
FINAL patch (Version 5) for calOutlookCSVImportExport.js

>  * Contributor(s):
>  *   Michiel van Leeuwen <[e-mail]>
>+ *   Ernst Herbst <[e-mail]>

I added me as contributor.


>@@ -80,26 +81,88 @@ const localeEn = {
>-    dateFormat      : "%m/%d/%y",
>+    dateFormat      : "%m/%d/%Y",

Export four digit years in english locale, see bug 366921.


>-    timeRe          : /^(\d+):(\d+):(\d+) (\w+)$/,
>+    timeRe          : /^(\d+):(\d+):(\d+)[ ]{0,1}(\w{0,2})$/,

Allow importing time values with missing AM/PM part. This is needed to read 24 hrs format. If user provides only 12 hrs format with missing  AM/PM part it is his fault. The code will red that and place all  events on AM. User can easily undo this import. Therefore a more liberal import check is better.


>-    timeFormat      : "%I:%M:%S %p"
>+    timeFormat      : "%H:%M:%S" // For 12 hour format change to "%I:%M:%S %p"

English is the default export setting. 24 hrs format is most commonly used and easier to handle, see bug 366921. Left comment in code for hardcore AM/PM users.


>+const localeDe = { ...
>+const localeFr = { ...

Added German and French locale


>@@ -114,38 +177,42 @@ const localeNl = {
>-    dateFormat      : "%d-%m-%y",
>+    dateFormat      : "%d-%m-%Y",

Was not used so far because export was only in english.


>-const locales = [localeEn, localeNl];
>+const locales = [localeEn, localeDe, localeFr, localeNl];

Widening array with new locales


>+// Change the following setting to your locale when applicable
>+const exportLocale = localeEn;
>+const exportLineEnding = "\r\n";

Introducing new constants for centralized export settings. Formerly the line ending was UNIX style which caused the outlook import to fail.


>@@ -171,34 +238,35 @@ function
>             var knownIndxs = 0;
>+            var needIndxs = 0;

Resetting the counters in locale loop.

>             for (var i = 1; i <= header.length; ++i) {
>                 switch( header[i-1] ) {
>-                    case locale.headTitle: args.titleIndex = i; knownIndxs++; break;
>- ...
>+                    case locale.headTitle: args.titleIndex = i; knownIndxs++; needIndxs++; break;

Increasing knownIndxs and the new variable needIndxs too. Lines were way longer than 80 chars. I'm not sure if I should have shortened these.

> ...
>                 }
>             }

End of the header loop inside the locale loop.


>-            if (knownIndxs >= 13)
>+            if (needIndxs >= 6)
>                 break;

Check if all necessary headers were recognized, see bug 337377 for the reduced amount of headers. If yes: Break out of locale loop.

>         }

End of locale loop


>@@ -207,20 +275,22 @@ function csv_importFromStream(aStream, a
>         if (knownIndxs == 0 && header.length == 22) {
>            // set default indexes for a default Outlook2000 CSV file
> ...
>+            knownIndxs = 13;
>+            needIndxs = 6;
>         }

The last resort fails in current code even if a valid outlook file was provided because knownIndxs was not set.


>-    // show field select dialog if not all required headers matched

That field select dialog in Sunbird 0.31 was not bad. Perhaps it should be reanimated as last resort. The XUL files are still in code. Deleting the comment for now.


>-        if (knownIndxs < 13) {
>+        // were enough columns recognized
>+        if (needIndxs < 6) {
>             dump("Can't import. Life sucks\n")

Checking if one of the four locales gave enough import headers or if a last resort outlook file was recognized.


>@@ -234,77 +304,102 @@ function csv_importFromStream(aStream, a
>-        args.boolStr = localeEn.valueTrue;
>-        args.boolIsTrue = true;
>-
>-        var dateParseConfirmed = false;

Deleting unused trash.


>-            var title = ("titleIndex" in args
>-                         ? 
parseTextField(eventFields[args.titleIndex]) : "");

The title field is parsed later on.


>-            var alarmDate = 
parseDateTime(eventFields[args.alarmDateIndex],
>- 
eventFields[args.alarmTimeIndex],
>-                                          locale);

The alarmDate is not a necessary field any more and may not be present in the file. It is parsed later on.


>-            if (title || sDate) {
>+            if (sDate) {

Check if we have a valid startDate and go on if yes. Otherwise skip to next line in import file.


> ...
>+                var title = 
parseTextField(eventFields[args.titleIndex]);
>+                if (title) {
>+                    event.title = title;
>+                } else {
>+                    event.title = "[" + locale.headTitle + "]";
>+                }

Take the header name in brackets if parsing of the event title fails.


> ...
>+                if (sDate.isDate){
>+                    eDate.isDate = true;
>+                }
>+                event.endDate = eDate;

Assure that startDate and endDate have the same isDay status.


>+                if ("alarmDateIndex" in args) {
>+                    if (locale.valueTrue == 
eventFields[args.alarmIndex]) {
>+                        var alarmDate = 
parseDateTime(eventFields[args.alarmDateIndex],
>+ 
eventFields[args.alarmTimeIndex],
>+                                                      locale);

Check if an alarm is wanted before calculating an alarmDate, see bug 401273.

>+                        if (!alarmDate) {
>+                            alarmDate = sDate.clone();
>+                            alarmDate.minute += -15; // 
?user_pref(calendar.alarms.eventalarmlen)
>+                        }
>+                        event.alarmOffset = 
alarmDate.subtractDate(sDate);
>+                        event.alarmRelated = 
Components.interfaces.calIItemBase.ALARM_RELATED_START;
>+                    }
>+                }

New logic for alarmDate. Subtract the startDate from the alarmDate to get a negative offset, see bug 405737. Set to 15 minutes before startDate if no valid alarmDate was parsed. I didn't use the user_pref for now.


>+                if ("privateIndex" in args) {
>+                    if (locale.valueTrue == 
eventFields[args.privateIndex]) {
>+                        event.privacy = "PRIVATE";
>+                    }
>+                }

Outlook takes the header title as true value for the private column.

>+ ...
>                 //save the event into return array
>                 eventArray.push(event);

The logic for importing fields was extensively changed. A detailed discussion of each line is inappropriate. Please see Marks comment on bug 366921#c14 as reference.


>@@ -320,49 +415,61 @@ function csv_importFromStream(aStream, a
> function parseDateTime(aDate, aTime, aLocale)
> { ...
>-    //XXX Can we do better?
>-    date.timezone = floating();

calendarDefaultTimezone() is used later on if a valid date was parsed.


>     var rd = aLocale.dateRe.exec(aDate);
>     var rt = aLocale.timeRe.exec(aTime);
>+    var rdYear = 0;

Introduced an integer var for dealing with two digit years.


>-    if (!rd || !rt) {
>+    if (!rd) {
>         return null;
>     }

Don't break if only a date could be parsed. Times are not needed for importing event lists for example. This events are set to all-day events in the calling function.


>-    date.year = rd[aLocale.dateYearIndex];
>+
>+    date.timezone = calendarDefaultTimezone();
>+    rdYear = Number(rd[aLocale.dateYearIndex])
>+    if (rdYear > 100) {
>+        date.year = rdYear;
>+    } else {
>+        // Most windows installations see two digit years in the 
range 1930 - 2029
>+        if (rdYear < 30) {
>+            date.year = 2000 + rdYear;
>+        } else {
>+            date.year = 1900 + rdYear;
>+        }
>+    }

Logic for handling two digit years.


>+        if (aLocale.timeAmPmIndex && rt[aLocale.timeAmPmIndex] != "") {

Process the AM/PM correction only if the import locale has a timeAmPmIndex AND if the regex got a result for this part of the time string.

>@@ -387,71 +494,83 @@ calOutlookCSVExporter.prototype.getFileT
> function csv_exportToStream(aStream, aCount, aItems) {
> ...
>-    headers.push(localeEn['headTitle']);
>- ...
>-    str = headers.map(function(v) {return '"'+v+'"';}).join(',')+"\n"
>+    headers.push(exportLocale['headTitle']);
>+ ...
>+    headers = headers.map(function(v) {return '"'+v+'"';});
>+    str = headers.join(',');
>+    str += exportLineEnding;

Taking the header titles from the chosen export locale, i.e. from the constant set in line 197. Simplifying the line "str = headers...". Taking the line ending from a constant.


>     for each (item in aItems) {
>-        var line = [];
>-        line.push(item.title);
>- ...
>+        if (item instanceof Components.interfaces.calIEvent) {
>+            var line = [];
>+            line.push(item.title);
>+ ...


Check if the calendar item is an event before processing it, see bug 336175.

>-        line.push(item.getProperty("CATEGORIES"));
>- ...
>-        line = line.map(function(v) {
>-            v = String(v).replace(/"/,'""');
>-            return '"'+v+'"';
>-        })
>- ...
>+            line.push(noNullString(item.getProperty("CATEGORIES")));
>+ ...
>+            line = line.map(function(v) {
>+                v = String(v).replace(/"/g,'""');
>+                return '"'+v+'"';
>+            })

Using the new function noNullString() to avoid "null" field values for empty categories, descriptions and locations in the exported csv file. Escaping all occurrences of double quotes, see bug 394634.


> function dateString(aDateTime) {
>-    return aDateTime.jsDate.toLocaleFormat(localeEn.dateFormat);
>+    return aDateTime.jsDate.toLocaleFormat(exportLocale.dateFormat);
> }
>
> function timeString(aDateTime) {
>-    return aDateTime.jsDate.toLocaleFormat(localeEn.timeFormat);
>+    return aDateTime.jsDate.toLocaleFormat(exportLocale.timeFormat);
>+}

Format exported dates and times as chosen in selected locale.


>+function noNullString(tstVar) {
>+    if (tstVar) {
>+        return tstVar;
>+    } else {
>+        return "";
>+    }
> }

New function for giving back an empty string instead of "null" as result for empty properties.

Current mozilla coding rules are still harmed after applying this patch. I'm unsure if reformatting parts of the code without changing their functionality is useful. Feel free to reject this version and I will provide a more stylish patch.

Comment 35

[:Hb]

Open questions from this and bug 173562 are:

Should the select calendar dialog be extended with the message: "While processing file X N columns where recognized. Do you want to import N events dating from MMDDYY to MMDDYY? [OK] [Cancel]".

Currently the select calendar dialog appears even if no events were recognized. This should not happen. More information for users about import failure reasons are needed.

Should duplicates be inserted in the calendar or should a dialog appear as in older versions? I tend to insert them without asking.

Should Jussis dialog form [1] be reactivated if not enough columns are recognized?

[1]
http://lxr.mozilla.org/mozilla/source/calendar/resources/content/outlookImportDialog.xul

Comment 36

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

Comment on attachment 299779 [details] [diff] [review]
FINAL patch (Version 5) for calOutlookCSVImportExport.js

>Index: calendar/import-export/calOutlookCSVImportExport.js
>+    timeFormat      : "%H:%M:%S" // For 12 hour format change to "%I:%M:%S %p"

12h format is the most common. (and i think that's what outlook uses for exports). So use that. And then it seems that the bug as filed isn't fixed. So either comment that you must use 24h format, or fix the real bug.

Comment 37

[:Hb]

Attachment: Eleven events export as CSV

All events from the sample calendar ElevenEvents (attachment 303900 [details]) after being exported as CSV file with version 5 of the patch.

Comment 38

[:Hb]

Attachment: Patch_v6 - Now using OS time and date formats

The 12/24 hrs issue is bigger as expected. Users in Great Britain mostly use DD/MM/YYYY and 24 hrs format while users in the U.S. tent to MM/DD/YYYY and 12 hrs AM/PM. In New Zealand and some South American Spanish speaking countries the indicators "a.m." and "p.m." have four chars and contain dots. Mongolia has only two digit years as default. Bulgaria has an abbrevation behind the year as last chars in the date format.

Generally we don't want to rebuild the localization of the operating system in Calendar. So import and export should use the settings of the OS.

Outlook handles the im- and export of datetimes accordingly to the OS. For export the format codes "%x" for date and "%X" for time and will (hopefully always [1] [2]) produce strings which outlook and spreadsheets can read. 

For import I didn't find an easy way to read the day-month-year order out of the OS. The following code is used to get this order from an OS-formatted  probe date: 
>
> +    probeDateTime.icalString = "20010203T161718";
> +    var rProbe = dateRe.exec(probeDateTime.jsDate.toLocaleFormat("%x"));
> +    for (var i = 1; i <= 3; ++i) {
> +        switch (Number(rProbe[i])) {
> +            case 1:                     // two digit years
> +                dateYearIndex  = i;	 break;
> +            case 2001:
> +                dateYearIndex  = i; 	 break;
> +            case 2:
> +                dateMonthIndex = i;   break;
> +            case 3:
> +                dateDayIndex   = i;   break;
> +    }   }

On the same way the PM indicator string is achieved.

Currently the localized column headers for DE, FR and NL are taken from this file. In the long run the should come out of the localization files. The corresponding settings for DE are on the way [3].


[1] http://de.php.net/manual/en/function.strftime.php
[2] http://msdn2.microsoft.com/library/fe06s4ak(vs.71).aspx.
[3] http://www.sunbird-kalender.de/forum/viewtopic.php?t=1880#8514

Comment 39

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

> Generally we don't want to rebuild the localization of the operating system in
> Calendar. So import and export should use the settings of the OS.

I disagree with that.
Before, the locale of the file was determined by the headers. Based on that, the time was parsed. That means that you can import an English csv file even if you have a different OS language. (ignoring the low number of locales for now).
With the proposed change, you can only import files in the locale of your OS.

The user doesn't have any influence of the language of the files he gets. It's really not said that it is always in the language of his OS. It must be possible to import all those files.

Comment 40

[:Hb]

Comment on attachment 304759 [details] [diff] [review]
Patch_v6 - Now using OS time and date formats 

(In reply to comment #39)
> > Generally we don't want to rebuild the localization of 
> > the operating system in Calendar. So import and export 
> > should use the settings of the OS.
>
> I disagree with that.
> Before, the locale of the file was determined by the headers. Based on that,
> the time was parsed. That means that you can import an English csv file even if
> you have a different OS language. ...

We have to distinguish between locale (MM/DD/YY, DD.MM.YYYY or YYYY-MM-DD and am/pm vs 24 hrs time) and the language of the column heads. An English Outlook set to it's default settings will produce an CSV file with English headers. But the date and time formats are based on OS settings: A user in the U.S. gets M/DD/YYYY, another one in Great Britain gets DD/MM/YYYY and the third one in South Africa will get YYYY/MM/DD. With a non English setting of the OS like nl-NL he will get English headers and dutch dates DD-MM-YYYY. Therefore I don't see the headers language as good source for a decision about locale. The patch for bug 301750 checks the time field for the presence of an am/pm indicator to decide if date format is medium endian (U.S.) or little endian (GB). That works for now, but can't be a solution on the long run.

> With the proposed change, you can only import files in the locale of your OS.

I differ with that. Files exported from Outlook on that system will always import well if header language fits.
 
> The user doesn't have any influence of the language of the files he gets. It's
> really not said that it is always in the language of his OS. It must be
> possible to import all those files.

In most of the cases users create the CSV files by them selfs whether with their copy of Outlook or with their spreadsheet programm.

I think QA advise is necessary here. The patch for bug 301750 should make it in the 0.8 version. For now I withdraw this patch. 


Key part of this patch is:

>+ const dateFormat = "%x";
>+ const timeFormat = "%X";
+

Comment 41

[:Hb]

Attachment: CSV file with YYYY-MM-DD date format

I'm asking for QA advice for the two main questions of CSV import/export: Header mapping and date/time formatting. 

Base information: Outlook produces CSV files in which the language of the column headers depends on the programs language version. The format of time and date data written to CSV files depends on the locale settings made by the user in the Regional Options control panel. Outlook reads times in 24 hour format correct even if the OS is set to am/pm format. 


Header Mapping: Import
----------------------
Sunbird 0.2 had a two step solution. First it was tried to match the CSV headers against the localized presets from Outlook which were stored in Sunbirds chrome locale properties. If that failed a dialog [1] was shown to let the user map the fields found in the CSV. The current release tries to recognize Outlooks localized header names in English and Dutch (+ German and French after review and check in of bug 301750) in the CSV file. If this fails a last resort attempt is made with counting the files column number. If it is 22 the column order is set for a default English outlook file regardless of the headers. The header names for the two (four) languages are hardcoded in calOutlookCSVImportExport.js.

A wanted feature is that Sunbird should be able to import files from foreign Outlook versions. This could be done with including the header names for more languages in the js file. This has two disadvantages. Localization data is stored in an unusual place. And Sunbird can only import files which header names fit exactly to Outlook. The solution under 0.2 was able to read more files, even if they came from an unknown Outlook version or from users spreadsheet program.

I propose a header mapping like under Sunbird 0.2:

1. Try the literal Outlook header names from the localization [2] to match 
   against the headers in the CSV file. 
2. If that fails: Test against the header names stored in the js file.
3. If that fails: Open a mapping dialog like most other CSV import routines 
   to as well.


Header Mapping: Export
----------------------
Header names should be taken from the localization [2].


Date/Time Formats: Import 
-------------------------
Currently the language of the CSV headers is used to determine the day - month - year order in dates. As shown in comment 40 this logic fails abroad the U.S. 

I propose the following solution:

1. If date values begins with a four digit number (= ISO 8601 standard) set 
   order to Y-M-D and continue with step 3.
2. Get the Y-M-D order from the operating system as shown in [3].
3. Get AM and PM indicator strings from the OS locale [4].

While importing all time values should be checked on the presence of OS based indicators and on the presence of the universal indicators "AM" and "PM". If a PM indicator is found and the hour is lower than 12 it is increased by 12. More expensive parsing is possible [5].

This routine is completely free of using localization informations from Sunbird which provides the best way for data exchange with Outlook.


Date/Time Formats: Export 
-------------------------
Currently (0.7/0.8 pre) data is exported only in en-US format MM/DD/YY with only two digit years and 12 hour-time format. Unfortunately the am/pm indicator is dropped under Windows (2000 and XP) if it is not defined in the Regional Options. This makes the export file worthless. 

Outlook, OpenOffice.org Calc and even Excel can import dates in YYYY-MM-DD (and YYYY/MM/DD) format *independent* of the locale settings or Regional Options of the OS. You may verify this with the attached CSV file [6]. 

I propose this date format and the 24-hour time format as defaults for exporting. They match the ISO 8601 standard and the Y-M-D order is easily seen. They provide a much better data exchange with external programs under all locales. 

To allow users to override the default the export process should look for the preferences "calendar.exportcsv.dateformat" and "...exportcsv.timeformat". 


Questions to Quality Assurance 
------------------------------
a) Should the field mapping dialog for import brought back to live?
b) Should YYYY-MM-DD and HH:MM:SS be used as default export format?


-----------
[1] SB 0.2 field mapping dialog (XUL!), see bug 173562 comment 34
https://bugzilla.mozilla.org/attachment.cgi?id=142445

[2] Literal Outlook headers
http://mxr.mozilla.org/mozilla/source/calendar/locales/en-US/chrome/calendar/calendar.properties#167

[3] Getting D-M-Y order from operating system with regexing a probe date.
>  probeDateTime = createDateTime();
>  probeDateTime.icalString = "20010203T161718";
>  var rProbe = dateRe.exec(probeDateTime.jsDate.toLocaleFormat("%x"));
>  for (var i = 1; i <= 3; ++i) {
>      switch (Number(rProbe[i])) {
>          case 1:       dateYearIndex  = i;   break;  // two digit years
>          case 2001:    dateYearIndex  = i;   break;
>          case 2:       dateMonthIndex = i;   break;
>          case 3:       dateDayIndex   = i;   break;
>      }
>  } ...

Mvl gave the good hint that the above code is only a subset of http://lxr.mozilla.org/mozilla/source/calendar/resources/content/datetimepickers/datetimepickers.xml?rev1.18#1586

[4] Get the locale dependent PM indicator as string and set index
>  timePmString    = timeRe.exec(probeDateTime.jsDate.toLocaleFormat("%X"))[4];
>  if (timePmString != "") 
>      timeAmPmIndex = 4;

[5] Parsing time data
http://lxr.mozilla.org/mozilla/source/calendar/resources/content/datetimepickers/datetimepickers.xml#1491

[6] File has the needed CRLF line ends to import it to outlook, see bug 419414

Comment 42

[Bas van den Bosch]

(In reply to comment #41)
> I propose a header mapping like under Sunbird 0.2:
> 
> 1. Try the literal Outlook header names from the localization [2] to match 
>    against the headers in the CSV file. 
> 2. If that fails: Test against the header names stored in the js file.

As these are ready to ship afaik and don't require additional strings I'd suggest to take these for now.

> 3. If that fails: Open a mapping dialog like most other CSV import routines 
>    to as well.

This would be nice for post 0.8

 
> 1. If date values begins with a four digit number (= ISO 8601 standard) set 
>    order to Y-M-D and continue with step 3.
> 2. Get the Y-M-D order from the operating system as shown in [3].
> 3. Get AM and PM indicator strings from the OS locale [4].

Think this is the right way to do this, but I wonder if there's more strings apart from AM and PM, are other indicators used? In other words, do you still need OS-locale? Afaik there's either 4-digit or AM/PM globally.

> Outlook, OpenOffice.org Calc and even Excel can import dates in YYYY-MM-DD (and
> YYYY/MM/DD) format *independent* of the locale settings or Regional Options of
> the OS. You may verify this with the attached CSV file [6]. 

If this is true, then let's take it. 



> To allow users to override the default the export process should look for the
> preferences "calendar.exportcsv.dateformat" and "...exportcsv.timeformat". 

We could but imho, this isn't even necessary. In the rare cases people might need this they could also use an editor (Calc, Excel) to alter the strings.  

> a) Should the field mapping dialog for import brought back to live?

post 0.8 would be great

> b) Should YYYY-MM-DD and HH:MM:SS be used as default export format?

see above...

You've done a lot of work of import/export. I haven't been able to keep track. I'd say, let's take the fixes and continue from there, marking the bugs which you worked on as fixed and file new bugs for the missing parts.

Comment 43

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

(In reply to comment #40)
> > With the proposed change, you can only import files in the locale of your OS.
> 
> I differ with that. Files exported from Outlook on that system will always
> import well if header language fits.
> 
The "on that system" part is key. You don't only import files from your own export, but maybe also downloaded files, files from your other computer which has a different OS, etc.

Comment 44

[:Hb]

Attachment: Patch v7

Comment 45

[:Hb]

Attachment: calOutlookCSVImportExport.js - Complete file for Sunbird/Lightning versions 0.7 and 0.8

Contains Patch_v7.

Comment 46

[:Hb]

Attachment: Patch_V8

Comment 47

[:Hb]

Attachment: calOutlookCSVImportExport.js - Complete file for Sunbird/Lightning versions 0.7 and 0.8

Comment 48

[:Hb]

Comment on attachment 306957 [details] [diff] [review]
Patch_V8

Import tests:
OS   Locale  File                              Result
---  ------  --------------------------------  --------------------
XP   de-DE   attachment 299545 [details]  Outlook_98_DE  ok
XP   de-DE   attachment 299545 [details]  Outlook_98_FR  ok
XP   de-DE   a. 299545  Outlook_2003_EN_Seven  am/pm time ok, dates 
                                               mismatched as expected
XP   de-DE   attachment 305849 [details]                 ok (YYY-MM-DD dates)

XP   en-US   attachment 299545 [details]  Outlook_98_DE  24 h time ok, dates mm. as exp.
XP   en-US   a. 299545        Outlook_2000_EN  ok
XP   en-US   attachment 305849 [details]                 ok (YYY-MM-DD dates)

XP   en-GB   attachment 299545 [details]  Outlook_98_DE  ok
XP   en-GB   a. 299545        Outlook_2000_EN  a/p time ok, dates mm. as exp.
XP   en-GB   attachment 305849 [details]                 ok (YYY-MM-DD dates)

Ubuntu: 
7.10 de-DE   attachment 299545 [details]  Outlook_98_FR  ok
7.10 de-DE   attachment 305849 [details]                 ok (YYY-MM-DD dates)
7.10 de-DE   a. 299545  Outlook_2003_EN_Seven  am/pm time ok, dates mm. as exp.


Export is OK on both systems.

See http://wiki.mozilla.org/Calendar:Import_Export for wanted behaviour.

Comment 49

[:Hb]

Attachment: CSV test file from french Outlook v2

Now with correct accents é and è.

Comment 50

[:Hb]

Umlauts and accents é and è are currently broken because of bugzillas bug 365926. Please use the US-ASCII charset for these attachments.

Comment 51

[Philipp Kewisch [:Fallen][☁️📆]]

Comment on attachment 306957 [details] [diff] [review]
Patch_V8

>+                switch (Number(rd[i])) {
>+                    case 01:   locale.dateYearIndex  = i;  break; // 2 digit yr
Not sure this will do what you want it to? 01 in this constellation is I believe an octal number. You probably want to drop the Number() in the switch?

>-                if (locale.valueTrue == eventFields[args.privateIndex])
>-                    event.privacy = "PRIVATE";
Is there no private field anymor?

>+        // Most windows systems see two digit years in the range 1930 - 2029
>+        if (rdYear < 30) {
>+            rdYear += 2000;
>+        } else {
>+            rdYear += 1900;
>+        }
Does this only happen for windows? If so, you could use
http://developer.mozilla.org/en/docs/Accessing_the_Windows_Registry_Using_XPCOM
to read those values from the registry (do cache those, if you decide to!)


>     var headers = [];
>     // Not using a loop here, since we need to be sure the order here matches
>     // with the orders the field data is added later on
>+    headers.push(locale['headTitle']);
>+    headers.push(locale['headStartDate']);
>+    headers.push(locale['headStartTime']);
>+    headers.push(locale['headEndDate']);
>+    headers.push(locale['headEndTime']);
>+    headers.push(locale['headAllDayEvent']);
>+    headers.push(locale['headAlarm']);
>+    headers.push(locale['headAlarmDate']);
>+    headers.push(locale['headAlarmTime']);
>+    headers.push(locale['headCategories']);
>+    headers.push(locale['headDescription']);
>+    headers.push(locale['headLocation']);
>+    headers.push(locale['headPrivate']);
I don't see why push() is needed here, as opposed to directly using

var headers = [ locale['headTitle'], ... ].map(function(v) {return '"' + v + '"'});

I know you didn't make it that way, but go ahead and change it.

>     for each (item in aItems) {
>+        if (!isEvent(item)) {
>+            continue;
>+        }
Tasks are not supported?

r=philipp with comments taken care of and questions answered

Comment 52

[Bas van den Bosch]

Hb, what is the status of the patch?

Comment 53

[Daniel Boelzle [:dbo]]

Hb, are you still working on this?

Comment 54

[:Hb]

Yes, hoping to be ready in November.

Comment 56

[Decathlon]

Attachment: CVS test from Italian Outlook 2003

Please Hb, could you add the Italian localization (Outlook 2003 version) to your patch?.
In the attachment there is a test case generated by Outlook 2003. The strings are these:

const locale_it = {
    headTitle       : "Oggetto",
    headStartDate   : "Data inizio",
    headStartTime   : "Ora inizio",
    headEndDate     : "Data fine",
    headEndTime     : "Ora fine",
    headAllDayEvent : "Giornata intera",
    headAlarm       : "Promemoria attivato/disattivato",
    headAlarmDate   : "Data promemoria",
    headAlarmTime   : "Ora promemoria",
    headCategories  : "Categorie",
    headDescription : "Descrizione",
    headLocation    : "Luogo",
    headPrivate     : "Privato",
    valueTrue       : "Vero",
    valueFalse      : "Falso",
};

Thanks a lot.

Comment 57

[Starko]

It seems that this issue has got sidetracked into some sort of outlook development center and then died out! 

How difficult can it be to write times that actually make sense?

Comment 58

[Philipp Kewisch [:Fallen][☁️📆]]

Attachment: Fix - v9

Debitrotted patch, for convenience. I'm sorry to say this doesn't mean I am working on this bug, but maybe someone else wants to fix the review comments from comment 51. Also contains the Italian locale.

If someone finds out a good way to retrieve the locale strings from the string bundle for locales other than the currently selected one, that would be even better.

Comment 59

[Heiko 'riot' Weinen]

Hello? Is this thing still on? I'm experiencing this and it is a major drawback. I'm looking for workarounds, but seeing how old this bug is, i'm a little bit puzzled. 

I'm not using _any_ outlook, the calendar i'm trying to export is a locally created .ics file.

Comment 60

[Zealot]

Any news on this issue?

Comment 61

[Martin Schröder [:mschroeder]]

The import/export code for Outlook CSV format has been removed in bug 1226851. Therefore resolving this bug report as WONTFIX.