Last Comment Bug 800333 - Event context menu propose to change attendance in a readonly calendar
: Event context menu propose to change attendance in a readonly calendar
Status: RESOLVED FIXED
:
Product: Calendar
Classification: Client Software
Component: Lightning Only (show other bugs)
: unspecified
: All All
: -- normal (vote)
: 1.9.1
Assigned To: Decathlon
:
Mentors:
Depends on: 886025
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-11 05:57 PDT by Michael Bailly
Modified: 2013-06-22 08:37 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch - v1 (2.05 KB, patch)
2012-10-27 03:47 PDT, Decathlon
philipp: review+
philipp: approval‑calendar‑beta+
philipp: approval‑calendar‑esr+
Details | Diff | Review
patch with corrections (2.03 KB, patch)
2012-11-06 03:23 PST, Decathlon
bv1578: review+
Details | Diff | Review

Description Michael Bailly 2012-10-11 05:57:11 PDT
When focusing on an event and right-clicking on it, the context menu allows me to change my attendance.
This choice should not be shown if the event is in a read-only calendar.
Comment 1 Felix Möller 2012-10-18 22:44:01 PDT
Thanks.

I can confirm this issue. Lightning 1.8 and thunderbird 16.0.1
Comment 2 Decathlon 2012-10-27 03:47:13 PDT
Created attachment 675823 [details] [diff] [review]
patch - v1

This patch fixes the bug, but I'm not sure about the desired behavior when two or more events with calendars writable and read-only are being selected at the same time. Should the "Attendance" menu item be enabled in that case?
I resolved making it enabled (the same for the "delete" menu item, although this case is a bit different) but in order to avoid an error that occurs when the change of attendance is being executed for events with non read-only calendars, it has to prevent executing the command for such events.

The way I've done it maybe is too simple but it solves this second issue and at the end of the command the items with read-only calendar still are selected and this could be useful as an indication to the user that the command has not be executed for those items, not sure if this behavior is right though.
Comment 3 Philipp Kewisch [:Fallen] 2012-11-02 09:06:01 PDT
Comment on attachment 675823 [details] [diff] [review]
patch - v1

Review of attachment 675823 [details] [diff] [review]:
-----------------------------------------------------------------

I think the way you put it is the best solution, otherwise the user might need to go hunting which items are readonly. r=philipp with one minor nit.

I'd like to see this on aurora and beta since its a good fix for enterprise calendars and the ESR is coming up. Could you cross-push it to all three trees?

::: calendar/base/content/calendar-item-editing.js
@@ +532,5 @@
>      startBatchTransaction();
>      try {
>          for each (let oldItem in items) {
> +            // Skip this item if its calendar is read only. 
> +            if (oldItem.calendar.getProperty("readOnly")) {

You can directly use oldItem.calendar.readOnly here. Also an extra whitespace in the comment line.
Comment 4 Decathlon 2012-11-06 03:23:06 PST
Created attachment 678682 [details] [diff] [review]
patch with corrections

I don't have an HG account yet (I know, I have to request it). I set the "checkin-needed" keyword.
Comment 5 Philipp Kewisch [:Fallen] 2012-11-06 09:10:37 PST
Pushed to comm-central changeset de55fd457665
Comment 6 Philipp Kewisch [:Fallen] 2012-11-28 09:01:56 PST
Comment on attachment 675823 [details] [diff] [review]
patch - v1

Oops I forgot to push this to beta/aurora. Since this is now on aurora anyway, I'm going to push to beta and release and add this to 1.9.1
Comment 7 Matthew Mecca [:mmecca] 2013-02-08 15:47:25 PST
comm-esr17 - https://hg.mozilla.org/releases/comm-esr17/rev/744ed601b22a

Note You need to log in before you can comment on or make changes to this bug.