Closed Bug 720263 Opened 8 years ago Closed 5 years ago

The imip-bar buttons status is not immediately updated after the response to the invitation

Categories

(Calendar :: E-mail based Scheduling (iTIP/iMIP), defect)

Lightning 1.1
defect
Not set

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: bv1578, Assigned: pookveeraya)

Details

Attachments

(1 file, 2 obsolete files)

Step to reproduce:
1. an email with an invitation has been received and the imip-bar in the tab mail
   shows the buttons "Accept", "Decline" and "Tentative";
2. select a response  -> TB send an email and the three buttons disappear;
3. select another email in the list;
4. select the previous mail -> the button "Open" appears in the imip-bar

Expected result:
the button "Open" should appear just after the response has been selected.

See bug 695247 comment 15
Assignee: nobody → pookveeraya
Status: NEW → ASSIGNED
feedback please?
Attachment #610666 - Flags: review?(philipp)
Comment on attachment 610666 [details] [diff] [review]
first patch - not so sure of variable naming convention

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

Just concerned about the part of opening of the openEventDialog, since it is within a callback, maybe before u set up the listener you can do this:
let self_ = self;
and then to call openEventDialog you can do this:
self_.openEventDialog(....//rest of the code.
Also the setAttribute thing needs to access the localization string.

Maybe these can of help: 
[1] https://developer.mozilla.org/en/Calendar/Localization
[2] https://wiki.mozilla.org/Calendar:Localization

Another version of the patch and it should be good to go :-)

::: calendar/lightning/content/imip-bar.js
@@ +183,5 @@
> +                                        aItemType,
> +                                        aDetail,
> +                                        aCount,
> +                                        aItems) {
> +                    if (aItems && aItems.length){

space after the parenthesis

@@ +211,5 @@
>                      let label = cal.itip.getCompleteText(aStatus, aOperationType);
>                      imipBar.setAttribute("label", label);
>  
> +                    let buttonElement = document.getElementById("imip-button1");
> +                    buttonElement.setAttribute("label", "Open");

Localization?
Attachment #610666 - Flags: review-
Comment on attachment 610666 [details] [diff] [review]
first patch - not so sure of variable naming convention

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

::: calendar/lightning/content/imip-bar.js
@@ +168,5 @@
>              if (items && items.length) {
>                  let item = items[0].isMutable ? items[0] : items[0].clone();
>                  modifyEventWithDialog(item);
>              }
> +        } else if (partStat == "SHOW") {

let self_ = self;

@@ +185,5 @@
> +                                        aCount,
> +                                        aItems) {
> +                    if (aItems && aItems.length){
> +                        let item = aItems[0].isMutable ? aItems[0] : aItems[0].clone();
> +                        modifyEventWithDialog(item);

self_.modifyEventDialog(item)
Attached patch edited patch (obsolete) β€” β€” Splinter Review
Call cal.calGetString("calendar", "Open"); instead of creating a new string in lightning.properties because of the following comment:
http://mxr.mozilla.org/comm-central/source/calendar/base/modules/calItipUtils.jsm#293
Attachment #610666 - Attachment is obsolete: true
Attachment #610666 - Flags: review?(philipp)
Attachment #612589 - Flags: review?(mohit.kanwal)
This patch creates a new label in calendar.properties for the Open button which get fetched using cal.calGetString("lightning", "imipOpen.label", null, "lightning");

I'm not sure if the "string freeze" mentioned in the comment (http://mxr.mozilla.org/comm-central/source/calendar/base/modules/calItipUtils.jsm#293) is still in effect so I attach both versions of the patch.
Attachment #612592 - Flags: review?(mohit.kanwal)
Comment on attachment 612589 [details] [diff] [review]
edited patch

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

Yup Looks good to me :-)
Not sure if the string freeze is still in effect.
@Fallen?
Attachment #612589 - Flags: review?(mohit.kanwal)
Attachment #612589 - Flags: review+
Attachment #612589 - Flags: feedback?(philipp)
Comment on attachment 612592 [details] [diff] [review]
another version of edited patch

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

Yup, I think this should be the way to do it. It would separate the UI string from the calendar code.
Attachment #612592 - Flags: review?(mohit.kanwal)
Attachment #612592 - Flags: review+
Attachment #612592 - Flags: feedback?(philipp)
Comment on attachment 612592 [details] [diff] [review]
another version of edited patch

Man I really need a combined request and feedback view for the whole project. Sorry for missing this one. I've put this patch in my review queue to make sure I get to it!
Attachment #612592 - Flags: feedback?(philipp) → review?(philipp)
And I'm going to take the checkin-needed out for now, since others might not know which patch to check. I'll do the checkin after looking into this patch.
Keywords: checkin-needed
Comment on attachment 612592 [details] [diff] [review]
another version of edited patch

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

r- to get the following fixed. This approach is generally better though.

::: calendar/lightning/content/imip-bar.js
@@ +168,5 @@
>              if (items && items.length) {
>                  let item = items[0].isMutable ? items[0] : items[0].clone();
>                  modifyEventWithDialog(item);
>              }
> +        } else if (partStat == "SHOW") {

Can't you reuse X-SHOWDETAILS here? They are very similar so I think it might make sense to collapse both into one. I'd prefer the version that gets the item from the calendar, this might also fix a few other issues when the item was changed. Please doublecheck if the original intent of X-SHOWDETAILS still works afterwards though.

@@ +173,5 @@
> +            let self_ = self;
> +            let listener = {
> +                onOperationComplete: function (aCalendar,
> +                                                aStatus,
> +                                                aOperationType,                                                                 

A few extra whitespaces here

@@ +193,5 @@
> +            }
> +
> +            let itemList = ltnImipBar.itipItem.getItemList({});
> +            ltnImipBar.itipItem.targetCalendar.getItem(itemList[0].id, listener);
> +            

And here
Attachment #612592 - Flags: review?(philipp) → review-
Attachment #612589 - Attachment is obsolete: true
Attachment #612589 - Flags: feedback?(philipp)
Please let me know if you are still interested in fixing this bug, I know its been quite a while.
This should not be an issue anymore since bug 990009. WFM with 3.9
Resolving to WFM as per comment #12. Feel free to reopen the bug if the issue still persists beyond 3.9
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.