Closed
Bug 367186
Opened 17 years ago
Closed 16 years ago
Migration wizard hangs when importing an empty ICS file from calendar extension
Categories
(Calendar :: Import and Export, defect)
Calendar
Import and Export
Tracking
(Not tracked)
RESOLVED
FIXED
0.8
People
(Reporter: cmtalbert, Assigned: ssitter)
Details
Attachments
(1 file, 2 obsolete files)
6.00 KB,
patch
|
dbo
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1 Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a2pre) Gecko/20070116 Calendar/0.6a1 The migration wizard hangs when it attempts to import an empty file from the calendar extension. This should simply generate an error and allow the migration wizard to proceed. Reproducible: Always Steps to Reproduce: 1. Install Calendar Extension into thunderbird, but do not add anything to its HOME calendar 2. Install Sunbird, launch using a clean profile 3. Migration wizard will appear, and offers to migrate from "Mozilla Calendar" 4. Allow migration wizard to proceed. Actual Results: Gets an error during migration that is caught by the Error Console: Error: [Exception... "Component returned failure code: 0x804a0100 [calIICSService.parseICS]" nsresult: "0x804a0100 (<unknown>)" location: "JS frame :: file:///Volumes/Calendar/Calendar.app/Contents/MacOS/js/calIcsParser.js :: ip_parseString :: line 60" data: no] Source File: file:///Volumes/Calendar/Calendar.app/Contents/MacOS/js/calIcsParser.js Line: 60 No errors are displayed to the user, the migration wizard's progress bar simply stops progressing and the wizard remains hung. Expected Results: The migration wizard should be able to handle this error under the covers and this should not hang the migration wizard. If you attempt this with the normal File->Import, it displays the error to the user and does not hang.
Comment 1•17 years ago
|
||
Since Mozilla Calendar 0.2 evidently used empty files for empty calendars, this should not be an error during migration; the empty calendar should just be silently imported as a calendar with no events in it. I ran into this same problem when migrating from Thunderbird 1.5 with Mozilla Calendar 0.2 to Thunderbird 2.0 with Lightning 0.5 -- as a result, half of my calendars weren't migrated, and I had to recreate them by hand.
Flags: blocking-calendar0.7?
Comment 2•17 years ago
|
||
Thats the relevant file for migration: http://lxr.mozilla.org/mozilla1.8/source/calendar/base/content/migration.js#603 (Function importICSToStorage) Thats what is used in import-export.js: 101 try 102 { 103 inputStream.init( fp.file, MODE_RDONLY, 0444, {} ); 104 105 var items = importer.importFromStream(inputStream, {}); 106 inputStream.close(); 107 } 108 catch(ex) 109 { 110 switch (ex.result) { 111 case Components.interfaces.calIErrors.INVALID_TIMEZONE: 112 showError(calGetString("calendar", "timezoneError", [filePath] , 1)); 113 break; 114 default: 115 showError(calGetString("calendar", "unableToRead") + filePath + "\n"+ ex); 116 } 117 } 118 119 if (aCalendar) { 120 putItemsIntoCal(aCalendar, items); 121 return; 122 }
Unfortunately we're too far into 0.7 to fix it at this point. We're going to have to just release note this one. But we'll fix it in 0.8.
Keywords: relnote
Comment 4•17 years ago
|
||
Added to 0.7 common release notes.
Updated•17 years ago
|
Flags: blocking-calendar0.7? → wanted-calendar0.8+
Assignee | ||
Comment 5•17 years ago
|
||
Patch adds the fix as suggested in Comment #2. During testing I noticed that the calendars created by the migration wizard are not visible in the calendar list although the creation was successful. An additional application restart is required to make them visible.
Assignee: nobody → ssitter
Status: NEW → ASSIGNED
Attachment #293285 -
Flags: review?(daniel.boelzle)
Assignee | ||
Comment 6•17 years ago
|
||
Mvl suggested on IRC to move the close() into a finally block. This patch iteration does this and adds the same change to import-export.js.
Attachment #293285 -
Attachment is obsolete: true
Attachment #293287 -
Flags: review?(daniel.boelzle)
Attachment #293285 -
Flags: review?(daniel.boelzle)
Comment 7•17 years ago
|
||
Comment on attachment 293287 [details] [diff] [review] add try-catch logic, rev1 looks good, thanks for the patch! just some nits: >Index: mozilla/calendar/base/content/migration.js ... >+ try { >+ inputStream.init(icsFile, MODE_RDONLY, 0444, {}); >+ var items = icsImporter.importFromStream(inputStream, {}); I like variables to be declared on the same level where used, i.e. outside the try-block. Init with [] so we always have a valid array, especially in case of an exception. >+ } finally { >+ inputStream.close(); >+ } Does close() work in case an exception has occurred executing init()? >Index: mozilla/calendar/base/content/import-export.js similar nits in this file r=dbo
Attachment #293287 -
Flags: review?(daniel.boelzle) → review+
Comment 8•16 years ago
|
||
Stefan, what is the status here? Can the patch be checked in?
Updated•16 years ago
|
Flags: wanted-calendar0.8+ → blocking-calendar0.8+
Assignee | ||
Comment 9•16 years ago
|
||
I'll provide an updated patch within the next days.
Assignee | ||
Comment 10•16 years ago
|
||
(In reply to comment #7) > Does close() work in case an exception has occurred executing init()? I assume it's safe to call but don't found any hint in the .idl descriptions. See <http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/mail/extensions/newsblog/content/feed-subscriptions.js&rev=1.21&mark=943-952#932> for a similar usage.
Assignee | ||
Updated•16 years ago
|
Keywords: checkin-needed
Comment 12•16 years ago
|
||
Patch checked into HEAD and MOZILLA_1_8_BRANCH.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Keywords: checkin-needed,
relnote
Resolution: --- → FIXED
Target Milestone: --- → 0.8
Updated•16 years ago
|
Attachment #295778 -
Flags: review?(daniel.boelzle)
Updated•16 years ago
|
Attachment #295778 -
Flags: review?(daniel.boelzle) → review+
You need to log in
before you can comment on or make changes to this bug.
Description
•