289803 - Edit events or tasks fails to save after 'ok' [trunk]

Status: RESOLVED FIXED

(Calendar :: Sunbird Only, defect)

Description

[gekacheka]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.7) Gecko/20050406 Firefox/1.0.3
Build Identifier: trunk

Editing existing event or task:
 o does not automatically select calendar
 o even if select calendar manually, does not close after 'ok'


In both cases problem is that more than one calendar object can be
created for the same calendar, so the two calendar objects are not ==.
Also, the calendar.uri objects are not ==.
Instead, suggest comparing the calendar.uri.spec strings.




Reproducible: Always

Steps to Reproduce:
1. Edit an existing event or task
2. Calendar is not selected in dialog, so select it.
3. click ok

Actual Results:  
Edit dialog did not close.

Another symptom:  JSConsole shows it is taking wrong branch

Warning: reference to undefined property originalEvent.parent.removeItem
Source File: chrome://calendar/content/calendar.js
Line: 978

Error: originalEvent.parent.removeItem is not a function
Source File: chrome://calendar/content/calendar.js
Line: 978


Expected Results:  
Saved and closed, no console errors.

Comment 1

[gekacheka]

Attachment: fix event dialog calendar default, calendar save

(patch -l -p 2 -i file.patch)

Changes

As mentioned, the problem was that the event.parent calendar was compared with
calendars in the list of calendars using ==.  Comparing respective calendar.uri
also fails.  This patch compares the respective calendar.uri.spec strings.

The event.parent calendar is null for new events.  In this case it should
default to the calendar that is selected in the calendars tab.	This patch
updates getSelectedCalendarOrNull to return the calendar object.  It also
documents where this value is passed that it is a calICalendar object, no
longer a server url.  The calendarEvent dialog now uses this calendar as the
default calendar when the event.parent is null.

In saveItem, the patch fixes the comparison between the new calendar and the
original calendar to compare the respective calendar.uri.spec.	Also, it fixes
removeItem --> deleteItem (calICalendar has a deleteItem method, not a
removeItem method).

Testing

Have at least 2 calendars available [may have to create in previous build]
Select a calendar, say file1.

'New Event' should open dialog and calendar should default to selected
calendar, file1.  Give a title, say test1.
'Ok' should successfuly save the event.

Select other calendar, say file2.

'New event' should open dialog and other calendar, file2, should be default.
Give it a title, say event2.  'Ok' should successfully save the event.

Edit first event, event1.  Dialog should show file1 as default calendar.
Change title, say to event1a.  Click 'ok'.  Event should successfully save.

Edit other event, event2.  Dialog should should file2 as default calendar.
Change title, say to event2a.  Click 'ok'.  Event should successfully save.

Edit event2 again.  Change to file1.  click ok.  Event should successfully
save.

Edit event2 again.  Observe that file is now file1.  click ok.

Repeat with tasks.

Comment 2

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

Comment on attachment 180295 [details] [diff] [review]
fix event dialog calendar default, calendar save

you can use uri.equals(otheruri)
r=mvl with that.

Comment 3

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

checked in patch with that change

Comment 4

[gekacheka]

MVL,

In the eventDialog change, you substituted
  if (event.parent.uri.equals(calendars[i].uri))
where the patch suggested
  if (eventCalendar && eventCalendar.uri.spec == calendars[i].uri.spec)
Event.parent may be null for new events, and eventCalendar is bound to the
supplied default args.calendar in that case, so the appropriate substitution
would be 
  if (eventCalendar && eventCalendar.uri.equals(calendar[i].uri))

Could you please fix?

Thanks for all your reviewing!

Comment 5

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

Done. Thanks for checking.
Hoping this is now really fixed :)