Closed Bug 415247 Opened 16 years ago Closed 16 years ago

Support all free/busy time types (FBTYPE) defined in rfc2445


(Calendar :: Calendar Frontend, enhancement)

Not set


(Not tracked)



(Reporter: nomisvai, Assigned: nomisvai)



(2 files, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20071127 Firefox/
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20080120 Calendar/0.8pre

Currently, only 3 colors are used to display availability of attendees: "free", "busy" and "No information". 

Our internal deployment shows that many users would like to also see the other FBTYPEs supported by RFC2445, which are defined here: 

        fbtypeparam        = "FBTYPE" "=" ("FREE" / "BUSY"
                           / "BUSY-UNAVAILABLE" / "BUSY-TENTATIVE"
                           / x-name

Would it be possible to add 2 new colors for "BUSY-UNAVAILABLE" and "BUSY-TENTATIVE" ? 

Reproducible: Always

Steps to Reproduce:
Also note that in my opinion, the freebusy view should be applied the following order:

4) BUSY/x-name

What I mean is that, for example, if a time range has both BUSY and BUSY-TENTATIVE, BUSY should win over BUSY-TENTATIVE...

Makes sense. API is already prepared for that (
Ever confirmed: true
Hardware: PC → All
Summary: The "invite attendees" dialog should support all fbtypes defined in rfc 2445 → Support all free/busy time types (FBTYPE) defined in rfc2445
Attached patch Add support for different FBTYPES (obsolete) β€” β€” Splinter Review
Please review carefully since this is my first patch with real code change. I would like to revisit the freebusy states represented as integers eventually, but I wanted to keep this patch small and easy to understand. 

I also changed the color of the "busy" state so it's similar with another popular calendar client:
free: invisible (not changed)
busy: dark blue (Changed from teal)
busy-tentative: Light blue (new)
busy-unavailable(our of office) : Purple (new)
unknown: red (not changed)

I will add a screenshot right after submitting this patch.
Attachment #355912 - Flags: ui-review?(christian.jansen)
Attachment #355912 - Flags: review+
Comment on attachment 355912 [details] [diff] [review]
Add support for different FBTYPES

>diff --git a/calendar/base/content/dialogs/calendar-event-dialog-attendees.xul b/calendar/base/content/dialogs/calendar-event-dialog-attendees.xul
>--- a/calendar/base/content/dialogs/calendar-event-dialog-attendees.xul
>+++ b/calendar/base/content/dialogs/calendar-event-dialog-attendees.xul
>@@ -125,18 +125,26 @@
>     </box>
>     <box orient="horizontal">
>       <box orient="vertical" flex="1">
>         <box orient="horizontal" align="center">
>           <box class="legend" status="FREE"/>
>           <label value="&;"/>
>         </box>
>         <box orient="horizontal" align="center">
>+          <box class="legend" status="BUSY_TENTATIVE"/>
>+          <label value="&event.freebusy.legend.busy_tentative;"/>
>+        </box>        
>+        <box orient="horizontal" align="center">
>           <box class="legend" status="BUSY"/>
>           <label value="&event.freebusy.legend.busy;"/>
>+        </box>
>+        <box orient="horizontal" align="center">
>+          <box class="legend" status="BUSY_UNAVAILABLE"/>
>+          <label value="&event.freebusy.legend.busy_unavailable;"/>
>         </box>
While you are at it, could you change all <box orient="horizontal"> to <hbox> and <box orient="vertical"> to <vbox> ?

Also, I'd probably prefer putting the last two legend items in a new column. Something like this:

[       ]  Free        [       ] Out of Office
[       ]  Tentative   [       ] No information
[       ]  Busy

Christian should decide on layout and colors though, requesting ui-review.

>               for each (var entry in aEntries) {
>                   var rangeStart = entry.interval.start.getInTimezone(kDefaultTimezone);
>                   var rangeEnd = entry.interval.end.getInTimezone(kDefaultTimezone);
>-                  var fbState = (entry.freeBusyType == Components.interfaces.calIFreeBusyInterval.UNKNOWN ? 0 : 2);
>+                  // fbState values are:
>+                  // 0 = Unavailable
>+                  // 1 = Free
>+                  // 2 = Busy
>+                  // 3 = Busy-Tentative
>+                  // 4 = Busy-Unavailable
>+                  var fbState = 0;
>+                  switch(entry.freeBusyType) {
>+                    case Components.interfaces.calIFreeBusyInterval.BUSY:
>+                      fbState = 2;
>+                      break;
>+                    case Components.interfaces.calIFreeBusyInterval.BUSY_TENTATIVE:
>+                      fbState = 3;
>+                      break;
>+                    case Components.interfaces.calIFreeBusyInterval.BUSY_UNAVAILABLE:
>+                      fbState = 4;
>+                      break;
>+                  }

let is the new var :-) Please change that for all variables inside the for each() block. Please fix indentation of the switch block we use 4-space indentation here. If you like, you could use a map instead, i.e:

let fbMap = {
    Components.interfaces.calIFreeBusyInterval.BUSY: 2,
    Components.interfaces.calIFreeBusyInterval.BUSY_TENTATIVE: 3,
    Components.interfaces.calIFreeBusyInterval.BUSY_UNAVAILABLE: 4

let fbState = fbMap[entry.freeBusyType] || 0;

It would be really great if you could file a followup bug (with a patch ;-) to get rid of that integer stuff!

r=philipp, please attach a patch with comments fixed.
Next time, please request review from someone on the module owner page ( )
Putting the legend items in one column also causes the dialog to be cut off for me. This can be fixed by

* changing the id of the attendees dialog (to reset persistance)
* removing width and height from the persist attribute on the window
* making sure the dialog is resizable (which it is)

I think we should do the above too to fix things in the long run. In case we keep the one-column layout, please do the above in your patch, otherwise its optional but appreciated.
Assignee: nobody →
Oh, and please do give yourself credit in the files you've changed.
Thanks for the review! Here is a new patch:
- Credited myself in the modified files
- Replaced box elements by vbox and hbox where appropriate
- Got rid of the "integer" stuff to keep the freebusy state, note that I could not define a map with the Components.interfaces.calIFreeBusyInterval.* values.
- Now using let instead of var in the functions I modified.
- Split the legend in 2 columns (See new Screenshot).
- Changed the id of the dialog (Note: I didn't find this pattern anywhere so I just added "-v2" at the end of the id, let me know if it's fine.
- The dialog will not be persisting the height and width anymore.
Attachment #355912 - Attachment is obsolete: true
Attachment #355912 - Flags: ui-review?(christian.jansen)
Attachment #355913 - Attachment is obsolete: true
Comment on attachment 356039 [details] [diff] [review]
Add support for different FBTYPES v2

ui-r=christian, based on an email we received. His comment:

"The two column layout is much better than the single one. We need to take care that the layout works with languages such like German, French or Portuguese."

I went ahead and consolidated a further box (made the dialog orient="vertical" and removed the outermost vbox) and made the legend not be equally spaced but aligned left. Thanks for taking care!

Pushed to comm-central <>

Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0
Target Milestone: 1.0 → 1.0b1
You need to log in before you can comment on or make changes to this bug.