Closed Bug 486678 Opened 11 years ago Closed 11 years ago

Calendar summary dialog is borked

Categories

(Calendar :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: dbo, Assigned: Fallen)

References

Details

(Whiteboard: [needed beta][no l10n impact])

Attachments

(1 file, 3 obsolete files)

- does not show attendees
- no alarms selectable
- custom alarms does not raise reminder dialog
Flags: blocking-calendar1.0?
Attached patch fix - v1 (obsolete) β€” β€” Splinter Review
Besides the fix, I've changed to allow directly choosing an alarm in case no DISPLAY alarms are supported (in read-write dialog).
for instance, WCAP currently only supports email alarms: choosing one of the direct list (non-custom) leads to adding a DISPLAY alarm which is filtered out, thus the alarm isn't saved. IMO it's OK to choose an email alarm in that list and save that. I'd prefer to show the email icon, too, but couldn't find out why it's not working.
Attachment #370858 - Flags: review?(philipp)
Flags: blocking-calendar1.0? → blocking-calendar1.0+
Whiteboard: [needed beta][no l10n impact][needs review]
Attached patch Fix - v2 (obsolete) β€” β€” Splinter Review
I've adapted your patch a bit to show an icon and also consolidated some code. This should fix the described issues.
Assignee: dbo.moz → philipp
Attachment #370969 - Flags: review?(dbo.moz)
Attachment #370858 - Attachment is obsolete: true
Attachment #370858 - Flags: review?(philipp)
Attachment #370969 - Flags: review?(dbo.moz) → review-
The patch doesn't work:
- WCAP alarms could not be set
- my default alarm settings are not recognized ("custom" is set), but clicking it there's no alarm...
Duplicate of this bug: 478476
Just wanted to make sure that this also references the missing Description field.  None of the other comments mention it.
Attached patch Fix - v3 (obsolete) β€” β€” Splinter Review
(In reply to comment #3)
> The patch doesn't work:
> - WCAP alarms could not be set
I could not reproduce, this worked for me.


> - my default alarm settings are not recognized ("custom" is set), but clicking
> it there's no alarm...
The fact that custom is selected is wrong, it should be "no reminders", since the default alarm should work for popup alarms only? We need real UI for default email alarms... or should we misuse the current UI for "an alarm for whatever the first alarm type is" ? For now I went with the misuse to not fully loose the default alarms for wcap.
Attachment #373872 - Flags: review?(dbo.moz)
Attachment #370969 - Attachment is obsolete: true
Attachment #373872 - Flags: review?(dbo.moz) → review-
Comment on attachment 373872 [details] [diff] [review]
Fix - v3

The patch works better now, default alarm settings are set and shown when creating a new event.

The only thing that does not yet work is storing an alarm in WCAP. I suspect that action type is still != EMAIL, thus filtered out in wcap:

    let alarms = item.getAlarms({}).filter(function(x) x.action == "EMAIL");

sorry, still r-
Attached patch Fix - v4 β€” β€” Splinter Review
Next try. I had the cache enabled, with which for some reason it worked. Disabling the cache exposed your error and this version fixes.
Attachment #373872 - Attachment is obsolete: true
Attachment #373910 - Flags: review?(dbo.moz)
Attachment #373910 - Flags: review?(dbo.moz) → review+
Comment on attachment 373910 [details] [diff] [review]
Fix - v4

patch works fine and looks good :)
r=dbo
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/6799f9b73982>

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0
Whiteboard: [needed beta][no l10n impact][needs review] → [needed beta][no l10n impact]
Target Milestone: 1.0 → 1.0b1
You need to log in before you can comment on or make changes to this bug.