401273 - alarms added to all events imported via csv file

Status: VERIFIED FIXED

(Calendar :: Import and Export, defect)

Description

[Ken Watson]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.8) Gecko/20071008 Firefox/2.0.0.8
Build Identifier: Lightning/0.7 within Thunderbird/2.0.0.6 (20070728)

After installing Lightning, I imported my calendar from Microsoft Outlook.  Every entry in Lightning had an alarm--most of them were 15 minutes, but many were random hours and days in advance.  I only use alarms on critical events, so now I need to go remove alarms from each event one at a time.  Very inconvenient and productivity-reducing.

Reproducible: Always

Steps to Reproduce:
1.  Export calendar file from Microsoft Outlook to .csv file.
2.  Import .csv file into Lightning.
3.  
Actual Results:  
Each event now carries an alarm.  Most are set for 15 minutes, but some are days and hours in advance.

Expected Results:  
No alarms, unless the original exported file included alarmed events.

Comment 1

[Stefan Sitter [:ssitter]]

Please attach the .csv file in question or post at least the header line and some lines with faulty events.

Comment 2

[Ken Watson]

Attachment: Header and first few lines of imported events in .csv file

I posted an extract.  Ken

Comment 3

[Philipp Kewisch [:Fallen][📱📆]]

Do you have the option enabled to add an alarm to new events by default? (tools/options/alarms)

Comment 4

[Stefan Sitter [:ssitter]]

The example .csv files contains disabled alarms for all events in the following format:

    ... ,Reminder on/off,Reminder Date,Reminder Time, ...
    ... ,FALSE,          11/8/2007,    12:45:00 PM    ...
    ... ,FALSE,          11/12/2007,   3:45:00 PM,    ...

From looking at the source file it's visible that the on/off setting is ignored and the alarm is added to the event as soon as an alarm date is available:
<http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/calendar/import-export/calOutlookCSVImportExport.js&rev=1.1.2.10&mark=266,292-295#265>

Comment 5

[Ken Watson]

(In reply to comment #3)
> Do you have the option enabled to add an alarm to new events by default?
> (tools/options/alarms)
> 
No, I ensured that the default I wanted was to NOT add an alarm.

Comment 6

[:Hb]

(In reply to comment #2)
> Header and first few lines of imported events in .csv file

The csv files exported from outlook I have seen so far have double quotes around each field, see https://bugzilla.mozilla.org/attachment.cgi?id=299344 

Your file looks like an csv file from Excel. These files are problematic because of unescaped field borders. In the German Excel version ; chars are field delimiters and are not escaped if they are in a cell data.

But as Stefan stated, the alarm on/off setting in the csv file is not considered. In bug 359083 is a replacement file which solves this issue.

Comment 7

[:Hb]

Attachment: Patch_v1 Looking first if alarm is wanted

Resolution:

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

Parse the alarmDate only if an alarm is wanted.

>@@ -284,19 +281,42 @@ function csv_importFromStream(aStream, a
> -      if (alarmDate) {
> -          event.alarmOffset = sDate.subtractDate(alarmDate);
> -          event.alarmRelated = Components.interfaces.calIItemBase
> -                               .ALARM_RELATED_START;

Don't set the alarmOffset generally if the alarmDate is valid. 

> +      // Exists an alarm true/false column?
> +      if ("alarmIndex" in args) {

The alarm column isn't mandatory any more. So first check if it is present.

> +          // Is an alarm wanted for this event?
> +          if (locale.valueTrue == eventFields[args.alarmIndex]) {

Proceed only if an alarm is wanted for this event.

> +              var alarmDate = parseDateTime( ... );
> +              // Set to default if non valid alarmDate was achieved
> +              if (alarmDate) {
> +                  event.alarmOffset = alarmDate.subtractDate(sDate);

Subtract the alarmDate from the startDate to get a negative duration for the offset. This resolves bug 405737.


Enhancement:

> +              } else {
> +                  var alarmOffset = Components ... .calIDuration);
> +                  var units = getPrefSafe("calendar.alarms
> +                                          .eventalarmunit","minutes");
> +                  alarmOffset[units] = getPrefSafe("calendar.alarms
> +                                                   .eventalarmlen",15);
> +                  alarmOffset.isNegative = true;
> +                  event.alarmOffset = alarmOffset;
> +              }
> +              event.alarmRelated = Components.interfaces.calIItemBase
> +                                             .ALARM_RELATED_START;

Take the preference settings for new alarms if an alarm is wanted but no alarm time is provided.

Comment 8

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

Comment on attachment 304930 [details] [diff] [review]
Patch_v1 Looking first if alarm is wanted

r=mvl

Comment 9

[Simon Paquet [:sipaq]]

Patch checked in on HEAD and MOZILLA_1_8_BRANCH.

--> FIXED

Comment 10

[:Hb]

Verified