394149 - [Proto] Event dialog: Status field (tentative, confirmed, canceled) is missing

Status: VERIFIED FIXED

(Calendar :: General, defect)

Description

[Patrick Strasser]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.1.6) Gecko/20070725 Firefox/2.0.0.6
Build Identifier: Lightning 0.7pre build 200708264 with Thunderbird Version 2.0.0.6 (20070728)

With the new event dialog I'm not able to edit the status (tentative, confirmed, cancelled) field. 


Reproducible: Always

Steps to Reproduce:
1. create a event.
2. try to set the status




The task dialog has a working status field.

Comment 1

[Stefan Sitter [:ssitter]]

Confirming. In my opinion this is a serious regression compared with the old dialog, e.g. what about existing events created with Sunbird 0.5 / Lightning 0.5 that have the status set? Requesting blocking‑calendar0.7.

Comment 2

[Patrick Strasser]

My old events with status are not affected. Status is not overwritten by the new dialog, but nonetheless it's not shown/editable.

Comment 3

[Daniel Boelzle [:dbo]]

IMO we need that feature, pointing to <http://tools.ietf.org/rfcmarkup?doc=2445> here for reference; l10n'ed strings are available.
Christian, could you please elaborate how that should be worked out in the event dialog?

Comment 4

[Christian Jansen]

Event Dialog Changes:

We should add the status stuff to the Options menu:

...
Timezone
--------
Importance >
Privacy    >
Status     >     Not specified  (Checked, by default)
--------         Tentative
Show Time as     Confirmed
                 Canceled


Event Summary Dialog Changes:

The "Status" information should be displayed (if Status = Tentative, or Confirmed, or Canceled) as the last entry above the first seperator. See attachment.

Comment 5

[Christian Jansen]

Attachment: Event Summary Dialog with Status information

Comment 6

[Michael Büttner (no reviews TFN)]

Attachment: patch v1

This patch adds the missing bits and pieces.

I also took the liberty to add the status values that tasks can come up with in the summary dialog (e.g. 'in progress', etc.).

Please also note that events don't get a default status which means that no menu entry is initially checked. This is not a problem of the dialog, but depends on how the events get constructed by default. This is basically the same underlying problem as in Bug 362926. All the dialog can do is to gracefully handle the distinction between undefined and "NONE" with respect to status (and privacy, and other properties that fall into this category).

Comment 7

[Christian Jansen]

UI-Review:
Tasks should not offer these status values, because the according values are already available (Status Drop Down). Please remove them.
See: http://tools.ietf.org/rfcmarkup?doc=2445#section-4.8.1.11

> Please also note that events don't get a default status which means that no
> menu entry is initially checked. This is not a problem of the dialog, but
> depends on how the events get constructed by default.

...but the "Status" drop down of the Task dialog offers a preselected item. In that case "Not Specified". This is not very consequent, or?

Comment 8

[Michael Büttner (no reviews TFN)]

(In reply to comment #7)
> Tasks should not offer these status values, because the according values are
> already available (Status Drop Down). Please remove them.
I added these labels to the summary dialog in order to display the status of tasks. Of course I didn't change anything in the task dialog.

> ...but the "Status" drop down of the Task dialog offers a preselected item. In
> that case "Not Specified". This is not very consequent, or?
The status won't be saved for tasks if the user doesn't explicitly sets anything. It just happens that for drop down lists we display the first entry, but this doesn't mean that the item really has this property set. It is just the natural consequence of using drop down lists and menu items. Furthermore, this needs to be handled in the backend, not in the dialog. I once had a discussion with daniel regarding defaults for our items, but even if we select sensible defaults it doesn't buy us much if we are confronted with a foreign item that doesn't have a status defined.

Comment 9

[Christian Jansen]

(In reply to comment #8)
> (In reply to comment #7)
> > Tasks should not offer these status values, because the according values are
> > already available (Status Drop Down). Please remove them.
> I added these labels to the summary dialog in order to display the status of
> tasks. Of course I didn't change anything in the task dialog.

I mean the task dialog, not the summary dialog.

Comment 10

[Michael Büttner (no reviews TFN)]

Attachment: patch v2

In order to summarize the previous discussion, the problem was that this patch also adds the new 'status'-menu erroneously to the task dialog as well as to the event dialog. Obviously it shouldn't be part of the menu in the task dialog. This symptom is only reproducible on a Mac, as strange as it may sound. I added a handler that dynamically removes the 'status'-menu from the tree to get rid of it in case we're editing a task. Everything else is just the same as with the previous patch. I hope this fixes the problem as I currently didn't have the chance to test this patch on a Mac.

Comment 11

[Christian Jansen]

Comment on attachment 280727 [details] [diff] [review]
patch v2

ui=christian. Works pefectly on Mac

Comment 12

[Daniel Boelzle [:dbo]]

Comment on attachment 280727 [details] [diff] [review]
patch v2

Flaws:
- The default selection ("Not specified") doesn't work for me for events.
- Resetting the status renders an invalid component, i.e.
...
SUMMARY:New Event
STATUS:<empty>
...

>+++ mozilla/calendar/base/content/calendar-summary-dialog.js	>+    if (item.hasProperty("STATUS")) {
IMO you could omit this hasProperty check since getProperty returns null.

>+++ mozilla/calendar/base/content/calendar-summary-dialog.xul	>+            <label value="&newevent.status.inprogress.label;" hidden="true" status="IN-PROCESS"/>
name it "inprocess".

>+++ mozilla/calendar/prototypes/wcap/sun-calendar-event-dialog.js	Thu Sep 13 >-    if (!isEvent(item)) {
>+    if (isEvent(item)) {
>+        if (gStatus || item.hasProperty("STATUS")) {
>+            item.setProperty("STATUS", gStatus);
>+        } else {
>+            item.deleteProperty("STATUS");
>+        }
>+    } else {
I don't understand the above:
- what about "NONE"?
- why call item.deleteProperty is item.hasProperty() is false?

r- for now

Comment 13

[Michael Büttner (no reviews TFN)]

Attachment: patch v3

Another try, I didn't recognize that 'NONE' isn't a valid value. This patch addresses this issue. Daniel, I didn't rename 'status.inprogress.label' since we're in string freeze. We already encountered some changes to the strings, but that will be handled after 0.7 is out of the door.

Comment 14

[Daniel Boelzle [:dbo]]

Comment on attachment 280871 [details] [diff] [review]
patch v3

>+++ mozilla/calendar/prototypes/wcap/sun-calendar-event-dialog.js	Fri Sep 14
>+            if (item.hasProperty("STATUS")) {
>+                item.deleteProperty("STATUS");
>+            }
no need for hasProperty()

works fine; r=dbo

Comment 15

[Michael Büttner (no reviews TFN)]

patch checked in on trunk and MOZILLA_1_8_BRANCH

-> FIXED

Comment 16

[Damian Szczepanik]

verified with
Mozilla/5.0 (Windows; U; Windows NT 5.1; pl; rv:1.8.1.8pre) Gecko/20070921 Calendar/0.7pre