Closed Bug 938455 Opened 11 years ago Closed 11 years ago

Items in Calendars not implementing calISchedulingSupport cannot be modified

Categories

(Calendar :: Dialogs, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: rkent, Assigned: rkent)

References

Details

Attachments

(1 file)

Due to a regression from bug 788004, calendars that do not implement calISchedulingSupport give this error when you try to open an event dialog:

JavaScript error: chrome://calendar/content/calUtils.js, line 250: aCalendar is
null

The root cause of this is this code in calendar-item-editing.js:

    calendar = cal.wrapInstance(calendar, Components.interfaces.calISchedulingSupport);
    if (calendar) {
        isInvitation = calendar.isInvitation(calendarItem);
    }
    // open the dialog modeless
    let url;
    if (isCalendarWritable(calendar)

because calendar is nulled out by the wrapInstance, and isCalendarWritable throws an error.

I assume this is unintentional?
Assignee: nobody → kent
Status: NEW → ASSIGNED
Attachment #831960 - Flags: review?(philipp)
Blocks: 788004
Comment on attachment 831960 [details] [diff] [review]
Don't overwrite the calendar

Looks good, sorry for the delay. r=philipp. I'm fine with approvals down to esr24 if you are quick. (c-c/c-a/c-b and comm-esr24)
Attachment #831960 - Flags: review?(philipp)
Attachment #831960 - Flags: review+
Attachment #831960 - Flags: approval-calendar-release+
Attachment #831960 - Flags: approval-calendar-beta+
Attachment #831960 - Flags: approval-calendar-aurora+
What calendar provider doesn't implement calISchedulingSupport? Knowing this would help testing and looking for regressions in the future.

Is this problem exposed in other places too? Could you try to open e.g. attendees dialog or edit dialog when calendar is marked as read-only? 

From a quick search e.g. the following places might be affected:
https://mxr.mozilla.org/comm-central/source/calendar/base/content/dialogs/calendar-event-dialog-attendees.js#619
https://mxr.mozilla.org/comm-central/source/calendar/base/content/dialogs/calendar-event-dialog-attendees.xml#116
https://mxr.mozilla.org/comm-central/source/calendar/base/content/dialogs/calendar-summary-dialog.js#54
"What calendar provider doesn't implement calISchedulingSupport"

In my case it was my experimental calendar support in ExQuilla for Exchange Web Services. It was easy enough to provide stub support though to work around this issue, so this is not an urgent need for me. I don't know enough about other providers to know whether this is an issue for them or not. But because I had traced it out, and the code change clearly introduced an unintended behavior change, I thought that I would declare the issue and its fix. In agreement with comment 3, I believe there are also other places where this issue occurs.

I'll go ahead and land this on comm-central, but as for release branches, I don't think that should be done unless 1) we determine this affects other providers, and 2) it is fixed in other places as well.
Checked in https://hg.mozilla.org/comm-central/rev/178eccc0971c

I'm going to resolve FIXED and ignore the release branch approvals, see my comment 4. But that does not mean I object, just that I don't know enough to justify doing branch landings. I don't need that for my extension.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
OS: Windows 7 → All
Hardware: x86_64 → All
Resolution: --- → FIXED
Target Milestone: --- → 3.0
Not sure about what you guys need to justify releasing a bugfix, but I struggled with this one as well. Agree on the easy workaround, though (once you know the issue). So while I don't need it for my extension either, it would be great if this could get landed at some time.

The other lines mentioned in comment 3 should be changed as well imho.

Oh, and slightly off-topic: http://mxr.mozilla.org/comm-central/source/calendar/providers/gdata/components/calGoogleUtils.js#588 has it the other way around; the second wrappedRItem should probably be a ritem or something. As I don't use the google stuff can't determine if the impact justifies a separate bug, though.
We just didn't push this to all branches. The patch has trickled down to beta, I've just pushed it to comm-esr24 so it will be in 2.6.5:

https://hg.mozilla.org/releases/comm-esr24/rev/e49d1ca3ab58
Target Milestone: 3.0 → 2.6.5
You need to log in before you can comment on or make changes to this bug.