320000 - event-dialog needs to work with the actual calAttendee object, not just the id

Status: RESOLVED FIXED

(Calendar :: Internal Components, defect)

Description

[Stefan Sitter [:ssitter]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20051212 Mozilla Sunbird/0.3a1+

Sunbird does not handle ATTENDEE property properly. It seems that only the email part is kept.

Step to reproduce:
1. import the testcase in Sunbird
2. Open event, change nothing and leave with Ok
3. export event and examine ics file

Before Sunbird the ics file contains:
ATTENDEE;CN="ScanApp Team";ROLE=REQ-PARTICIPANT;RSVP=TRUE:MAILTO:[e-mail]

After Sunbird the ics file contains: ATTENDEE:MAILTO:[e-mail]
(After Lightning the ics file contains: ATTENDEE:mailto:undefined)

Also the ORGANIZER property is completely disappeared.

Comment 1

[Stefan Sitter [:ssitter]]

Attachment: testcase

Comment 2

[Joey Minta]

Changing summary to reflect that this is a front-end bug, not back-end.

The serializing and unserializing works flawlessly, (according to liberal use of dump()).  The problem is that the event dialog does not actually use the calAttendee object that has all the data.  Rather, it just grabs the .id, discards the object, and then creates a new object in the onok function.  Instead, it should keep that object around, and only change the parameters that it exposes.

Comment 3

[Joey Minta]

Attachment: find existing attendees first

This patch fixes the dataloss issues in a very basic way.  What we really need is probably to expose a lot more UI for some of these extra fields.  However, as long as the person doesn't change the email address, the other fields will remain.  If the person does change the email address, then I can't see them expecting the extra fields to stay as well, since they're invisible.

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 206697 [details] [diff] [review]
find existing attendees first

Looks good!

>     /* attendence */
>-    item.removeAllAttendees();
>     var attendees = getElementValue("item-attendees");
>+    var newAtts = new Array();
>     if (attendees != "") {
>         for each (var addr in attendees.split(",")) {
>             if (addr != "") {
>-                var attendee = createAttendee();
>-                attendee.id = "mailto:" + addr;
>-                item.addAttendee(attendee);
>+                var found = false;
>+                // Look to see if this attendee was around from before
>+                for each (var attendee in item.getAttendees({})) {
>+                    if (found)
>+                        continue;
>+                    if (attendee.id.split("MAILTO:")[1] == addr) {
>+                        newAtts.push(attendee);
>+                        found = true;
>+                    }

Why not just add a "continue" statement after setting found=true?  Then you wouldn't need the first if.

r=dmose with that change.

Can you open a new bug about the remaining dataloss here?  You're right that it requires more UI, and I don't think it's worth blocking 0.1 for.

Thanks!

Comment 5

[Joey Minta]

Comment on attachment 206697 [details] [diff] [review]
find existing attendees first

I'm going to hold off checking in this patch.  It conflicts with the larger changes to the dialog that I want to make.  Furthermore, those larger changes would make this bug/patch obsolete.

Comment 6

[Joey Minta]

Fixed by the dialog unification (bug 296893)