1283623 - [GSoC 2016] Fix issues in the UI for editing events/tasks in a tab

Status: RESOLVED FIXED

(Calendar :: Dialogs, defect)

Description

[Paul Morris [:pmorris]]

This bug tracks follow-up work on the initial implementation of editing events/tasks in a tab from bug 1277972. 

Known issues to fix:

- file > quit closes a tab rather than quitting the app
- today pane state is not preserved when switching to/from event/task tabs
- view menu > toolbars: include the event in a tab toolbar when in an event tab
- make sure all relevant menu items work for events/tasks in a tab
- event/task specific menu items in "Events and Tasks" menu are disabled
- when customizing toolbar, disable menu items
- tabs need icons for event and task tabs
- toolbar needs "hamburger" app menu in tab (calendar-appmenu-button)
- toggle timezones button on/off to match timezones shown/hidden when switching tabs

Comment 1

[Paul Morris [:pmorris]]

Attachment: disable-menu-items-aug11.patch

When editing an event/task in a tab, and customizing the toolbar, disable all menu items.  Once the customization is done, enable the items that were enabled before the customization began.

Comment 2

[Paul Morris [:pmorris]]

Attachment: toolbars-menu-integration-aug12.patch

When editing an event/task in a tab, show the event toolbar in the View > Toolbars menu and let the View > Toolbars > Customize menu item customize the event toolbar.  This patch passes mozmill tests.

Comment 3

[[:MakeMyDay]]

Can you please wrap up, which of the issues from comment 0 are still subject of this bug? Some should be resolved already in bug 1277972.

Comment 4

[[:MakeMyDay]]

Comment on attachment 8780292 [details] [diff] [review]
disable-menu-items-aug11.patch

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

r- for now, please check the comments below.

::: calendar/lightning/content/lightning-item-panel.js
@@ +801,1 @@
>      if (menubar) {

Why is the if block still needed here, now that you support both modes? Also, please leave the comment here to explain the purpose.

@@ +831,1 @@
>      if (menubar) {

Same question as above.

Comment 5

[Paul Morris [:pmorris]]

(In reply to [:MakeMyDay] from comment #3)
> Can you please wrap up, which of the issues from comment 0 are still subject
> of this bug? Some should be resolved already in bug 1277972.

Four are still left to be resolved under this bug.  Patches are attached for 1 and 2, and I'm working on 3.  I expect 4 will be done when 3 is done.  The rest were already resolved in bug 1277972.  (And it turns out the today pane behavior was a feature not a bug.)

1. when customizing toolbar, disable menu items (patch attached)

2. view menu > toolbars: include the event in a tab toolbar when in an event tab (and let 'Customize' customize the correct toolbar) (patch attached)

3. event/task specific menu items in "Events and Tasks" menu are disabled (when an event/task is open in a tab, enable them and make them work on that task/event).

4. make sure all relevant menu items work for events/tasks in a tab

Comment 6

[Paul Morris [:pmorris]]

Attachment: disable-menu-items-aug13.patch [checked-in - see comment 17]

- Removes "if (menubar) {...}" conditionals.
- Keeps explanatory comments.
- Changes 'for' loops to 'for of' loops (while we're at it).

Comment 7

[[:MakeMyDay]]

Comment on attachment 8780832 [details] [diff] [review]
disable-menu-items-aug13.patch [checked-in - see comment 17]

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

looks good, r=me.

Comment 8

[[:MakeMyDay]]

Comment on attachment 8780664 [details] [diff] [review]
toolbars-menu-integration-aug12.patch

This patch seems to regress the toolbar customization capabilities from view > toolbar in mail mode. In that mode, there are no toolbars displayed above the customize menu item.

Also, it seems to trigger a mozmill failure for autohiding the menu bar (on Windows) - see [1]. To be sure this behaviour is not influenced by the html patch, which I pushed on top, I triggered [2].

It might be a problem here that you don't respect the broadcasted tab mode anymore. You should modify you approach to define separate modes for event and task in a tab (which also would help you to resolve the today pane issue, which is still one - you currently seem to share the mode for these with any mail mode, which makes it impossible to show/adpat the today pane e.g. in mail mode but not in event tab mode) and use that in the toolbar customization code.

Not related to this patch, but maybe for patch no 4 or even better a follow up bug, we're currently lacking the option to just save and event/task without closing in tab mode. As currently the only option to save is the save & close button, we would probably need menu items in the file menue to allow for save & close, save and delete as it is available in window mode. 


[1] https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=86faa1b21bd5db83d9f19f31da967b04b28b470a
[2] https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=1dbf1f0d96745ba578f40cfd6bbce840ab74cecd

Comment 9

[Paul Morris [:pmorris]]

Attachment: events-tasks-menu-items-aug14.patch

This patch adds support for these "Events and Tasks" menu items for task tabs: Mark completed, Priority, Progress, Postpone Task.

(The Priority menu item could/should be enabled for event tabs as well, but I have left this for a later patch, if needed.)

Comment 10

[Paul Morris [:pmorris]]

(In reply to [:MakeMyDay] from comment #8)
> Comment on attachment 8780664 [details] [diff] [review]
> toolbars-menu-integration-aug12.patch
> 
> This patch seems to regress the toolbar customization capabilities from view
> > toolbar in mail mode. In that mode, there are no toolbars displayed above
> the customize menu item.

Hmm... on linux, in mail mode, under view > toolbars, this patch shows Quick Filter Bar, Status Bar, and Customize.  I had noticed the Menu Bar was missing in mail mode, but the code seemed to be omitting it on purpose for some reason (where "navigation-toolbox" is only pushed to the array of toolboxes when not in mail mode).  So I followed the existing code.

I now see that there must be another mechanism for populating this menu (with Menu Bar and Mail Toolbar) that this patch disrupts.  (Which explains the reason for omitting "navigation-toolbar" for mail mode, because it should already be there.)

> Also, it seems to trigger a mozmill failure for autohiding the menu bar (on
> Windows) - see [1]. To be sure this behaviour is not influenced by the html
> patch, which I pushed on top, I triggered [2].

Ok, good to know.  Thanks for pushing those try server runs.

> It might be a problem here that you don't respect the broadcasted tab mode
> anymore. You should modify you approach to define separate modes for event
> and task in a tab (which also would help you to resolve the today pane
> issue, which is still one - you currently seem to share the mode for these
> with any mail mode, which makes it impossible to show/adpat the today pane
> e.g. in mail mode but not in event tab mode) and use that in the toolbar
> customization code.

So far I had avoided using this mode system because early this summer Philipp mentioned that it would be good to eventually move away from using it.  It would be good to have his input before proceeding, but I haven't been able to reach him on IRC yet.

Good catch on that todaypane issue.  I've put it on my todo list and can create a new bug for it.

> Not related to this patch, but maybe for patch no 4 or even better a follow
> up bug, we're currently lacking the option to just save and event/task
> without closing in tab mode. As currently the only option to save is the
> save & close button, we would probably need menu items in the file menue to
> allow for save & close, save and delete as it is available in window mode.

Yes, this would be good to have.  I can create a follow-up bug for it.  It might be good to have a "Save" toolbar button as well?  Although I remember you had some related work in progress for the current "Save and Close" button?

Comment 11

[Paul Morris [:pmorris]]

Attachment: toolbars-menu-integration-aug14.patch

This patch fixes the issues with the previous version.

Comment 12

[[:MakeMyDay]]

Comment on attachment 8780955 [details] [diff] [review]
toolbars-menu-integration-aug14.patch

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

No inline comments this time, because splinter doesn't work for that from my mobile.

So first, does this also work when you have opened the chat tab? Or do we need a more general fallback when it's not a calendar toolbox?

Then, each time you use getToolboxIdForCurrentTabType, you apply the fallback to mail-toolbox. If you need this always move that into the function as a general fallback instead of null.

At some places you use single instead of double quotes where they are not needed. Can you fix this, please?

And finally, can you please trigger a try build for Windows including mozmill tests - that platform seems to be the one to reliable have the autohide test failure.

Comment 13

[[:MakeMyDay]]

Try push: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=35048334b83af1cda115e2252966db86f3dd0903

Comment 14

[Paul Morris [:pmorris]]

Attachment: toolbars-menu-integration-aug15.patch

> So first, does this also work when you have opened the chat tab? Or do we 
> need a more general fallback when it's not a calendar toolbox?

On the current release (for linux), View > Toolbars in a chat tab, is the same as View > Toolbars in a mail tab (menu, mail, folder, quick, status, customize).  And this patch does not change that.

Looks like we are about to surpass the chat implementation's level of integration with the View > Toolbars menu.

> Then, each time you use getToolboxIdForCurrentTabType, you apply the fallback 
> to mail-toolbox. If you need this always move that into the function as a 
> general fallback instead of null.

Done.

> At some places you use single instead of double quotes where they are not 
> needed. Can you fix this, please?

Done.

> And finally, can you please trigger a try build for Windows including mozmill 
> tests - that platform seems to be the one to reliable have the autohide test 
> failure.

Ok, I'll see if I can get it to work for me this time.

Comment 15

[[:MakeMyDay]]

Comment on attachment 8781219 [details] [diff] [review]
toolbars-menu-integration-aug15.patch

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

Just one nit. r+ provided you upload an updated patch and the test of the try run passes.

::: calendar/lightning/content/messenger-overlay-sidebar.js
@@ +793,5 @@
>  
>  /**
> + * Get the toolbox id for the current tab type.
> + *
> + * @return {string|null}  A toolbox id or null

This is string only now.

Comment 16

[Paul Morris [:pmorris]]

Attachment: toolbars-menu-integration-aug15-2nd.patch [checked-in - see comment 17]

> + * @return {string|null}  A toolbox id or null
>
> This is string only now.

Fixed. Thanks for catching that and for starting the try server run.

Comment 17

[[:MakeMyDay]]

https://hg.mozilla.org/comm-central/rev/30dff50b5b66647581cf282062a516ec32f4706b
https://hg.mozilla.org/comm-central/rev/e7efeb32a0ea919b073d5c1959fe6caa5db6f83c

Comment 18

[Jorg K (CEST = GMT+2)]

Can you please look into these Mozmill test failures (soon to appear on C-C):
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=4dd3b3777d9c893fa3d881ef5efcd6da8a0554ca

06:55:49     INFO -  SUMMARY-PASS | testLocalICS.js::setupModule
06:55:49     INFO -  SUMMARY-PASS | testLocalICS.js::teardownTest
06:55:49     INFO -  SUMMARY-UNEXPECTED-FAIL | testLocalICS.js | testLocalICS.js::testLocalICS
06:55:49     INFO -    EXCEPTION: Menu entry '#ltnNewCalendar' not found.
06:55:49     INFO -      at: controller.js line 223
06:55:49     INFO -         Menu.prototype.getItem controller.js:223 13
06:55:49     INFO -         Menu.prototype.click controller.js:238 28
06:55:49     INFO -         testLocalICS testLocalICS.js:43 3
06:55:49     INFO -         Runner.prototype.wrapper frame.js:585 9
06:55:49     INFO -         Runner.prototype._runTestModule frame.js:655 9
06:55:49     INFO -         Runner.prototype.runTestModule frame.js:701 3
06:55:49     INFO -         Runner.prototype.runTestFile frame.js:534 3
06:55:49     INFO -         runTestFile frame.js:713 3
06:55:49     INFO -         Bridge.prototype._execFunction server.js:179 10
06:55:49     INFO -         Bridge.prototype.execFunction server.js:183 16
06:55:49     INFO -         Session.prototype.receive server.js:283 3
06:55:49     INFO -         AsyncRead.prototype.onDataAvailable server.js:88 3

Comment 19

[[:MakeMyDay]]

Comment on attachment 8781260 [details] [diff] [review]
toolbars-menu-integration-aug15-2nd.patch [checked-in - see comment 17]

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

Paul, can you please prepare a patch to fix the below issue? Thanks.

::: calendar/lightning/content/lightning-menus.xul
@@ +476,5 @@
>                 insertafter="appmenu_newAccountMenuItem"/>
>    </menupopup>
>    <splitmenu id="appmenu_customize">
>      <menupopup id="appmenu_customizeMenu"
> +               onpopupshowing="onToolbarsPopupShowingForTabType(event, document.getElementById("appmenu_quickFilterBar"));"/>

The problem is this change, which results in a malformed xul and prevents the lightning menu to be displayed at all.

Comment 20

[[:MakeMyDay]]

Fixing here just means reverting this line to what it was before.

Comment 21

[Jorg K (CEST = GMT+2)]

Thanks, pushed a fix for that:
https://hg.mozilla.org/comm-central/rev/bde9f7838d8c

Comment 22

[[:MakeMyDay]]

Comment on attachment 8780918 [details] [diff] [review]
events-tasks-menu-items-aug14.patch

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

r- to get an updated patch picking up the comments below (almost r+ already, but I would like you to check on the type conversion thing for percentComplete).

::: calendar/base/content/calendar-menus.xml
@@ +43,5 @@
>                  }
>              }
> +
> +            if (gTabmail.currentTabInfo.mode.type == "calendarTask") {
> +                let command = document.getElementById("calendar_" + this.mType + "-" + gConfig[this.mType] + "_command");

Please define the command variable outside of the if block, so it doesn't need to de defined twice in if and else block. If you do so, you subsequently also can move the if (command) {...} block to the outside to de-duplicate that code.

@@ +47,5 @@
> +                let command = document.getElementById("calendar_" + this.mType + "-" + gConfig[this.mType] + "_command");
> +                if (command) {
> +                    command.setAttribute("checked", "true");
> +                }
> +

Please remove this line

::: calendar/base/content/calendar-task-tree.js
@@ +120,3 @@
>   *
> + * @param {XULCommandEvent} aEvent  The DOM event that triggered this command
> + * @param {number} aProgress        The new progress percentage

Please be precise if possible when it comes to number types. percentComplete is a short as yo can see in calIToDo.idl

@@ +123,5 @@
>   */
>  function contextChangeTaskProgress(aEvent, aProgress) {
> +    if (gTabmail.currentTabInfo.mode.type == "calendarTask") {
> +        editToDoStatus(aProgress);
> +

Please spare this line.

@@ +173,3 @@
>   *
> + * @param {XULCommandEvent} aEvent  The DOM event that triggered this command
> + * @param {number} aPriority        The priority to set on the task(s)

Also priority is of type short (see calIItemBase.idl)

@@ +177,5 @@
>  function contextChangeTaskPriority(aEvent, aPriority) {
> +    let tabType = gTabmail.currentTabInfo.mode.type;
> +    if (tabType == "calendarTask" || tabType == "calendarEvent") {
> +        editPriority(aPriority);
> +

Spare the line.

@@ +202,5 @@
>  function contextPostponeTask(aEvent, aDuration) {
>      let duration = cal.createDuration(aDuration);
>      if (!duration) {
>          cal.LOG("[calendar-task-tree] Postpone Task - Invalid duration " + aDuration);
>      }

We should return in this case directly. Passing an invalid duration object doesn't seem to make sense.

@@ +205,5 @@
>          cal.LOG("[calendar-task-tree] Postpone Task - Invalid duration " + aDuration);
>      }
>  
> +    if (gTabmail.currentTabInfo.mode.type == "calendarTask") {
> +        postponeTask(aDuration);

why don't you pass duration here? This would save a type conversion later on in postponeTask()

@@ +227,5 @@
>  /**
>   * Modifies the selected tasks with the event dialog
>   *
>   * @param aEvent        The DOM event that triggered this command.
>   * @param initialDate   (optional) The initial date for new task datepickers  

Please remove the whitespace at the end of the line here.

::: calendar/lightning/content/lightning-item-iframe.js
@@ +2697,2 @@
>   */
> +function updateToDoStatus(aStatus, aPassedInCompletedDate) {

Can you please rename aPassedInCompletedDate to aCompleteDate?

@@ +2702,5 @@
>    // back to COMPLETED). When we go to store this VTODO as .ics the
>    // date will get lost.
>  
>    // remember the original values
>    let oldPercentComplete = getElementValue("percent-complete-textbox");

can you check whether you can make this parseInt(getElementValue("percent-complete-textbox")) and get rid of making the numeric value a string subsequently?

@@ +2719,4 @@
>        }
>    }
>  
> +  let completedDate = aPassedInCompletedDate ? aPassedInCompletedDate : null;

You simply can assign a default value of null above and then use the argument directly subsequently:

function updateToDoStatus(aStatus, aCompletedDate=null) {

@@ +2762,1 @@
>         oldPercentComplete == "100") {

please reduce the intentation by one whitespace here.

@@ +2987,5 @@
> + *
> + * @param {string} aDuration  A duration in ISO 8601 format
> + */
> +function postponeTask(aDuration) {
> +    let duration = cal.createDuration(aDuration);

As mentioned above, make this to consume a calIDuration object instead of a string.

::: calendar/lightning/content/lightning-item-panel.js
@@ +496,4 @@
>   */
>  function editPriority(aTarget) {
> +    let newPriority = typeof aTarget == "number" ?
> +        aTarget : parseInt(aTarget.getAttribute("value"));

Can you please avoid this type mixture? Either make this two functions, one consuming a node object and a second consuming the short, which then can be used by the first and also directly from within contextChangeTaskPriority, or change all of the other consumers to provide a numeric argument instead of a node object.

@@ +670,5 @@
>  
>  /**
> + * Change the task percent complete (and thus task status).
> + *
> + * @param {number} aTarget  The new percent complete value

short

@@ +686,5 @@
> + * @param {string} aArg.percentComplete  The percent complete value as a string
> + */
> +function updateMarkCompletedMenuItem(aArg) {
> +    let completedCommand = document.getElementById("calendar_toggle_completed_command");
> +    let isCompleted = aArg.percentComplete == "100";

If you have been changing updateToDoStatus to pass a integer instead of a string as requested above, this would need to be modified accordingly.

@@ +694,5 @@
> +/**
> + * Postpone the task's start date/time and due date/time. ISO 8601
> + * format: "PT1H", "P1D", and "P1W" are 1 hour, 1 day, and 1 week.
> + *
> + * @param {string} aDuration  A duration in ISO 8601 format

As above, this should expect an calIDuration object.

Comment 23

[[:MakeMyDay]]

Comment on attachment 8780918 [details] [diff] [review]
events-tasks-menu-items-aug14.patch

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

See the below comments related to the mozmill test failure here: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=447d3a5cd8bd054d9752e04ab635b4c62377122d

::: calendar/base/content/calendar-menus.xml
@@ +43,4 @@
>                  }
>              }
> +
> +            if (gTabmail.currentTabInfo.mode.type == "calendarTask") {

gTabmail doesn't seem to be defined in each context, which prevents the menue to be displayed correctly and probably triggers the mozmill test failure. Maybe it's enough to add an additional check on whether gTabmail exists, but it would be better to understand why it's not available sometimes.

::: calendar/base/content/calendar-task-tree.js
@@ +123,3 @@
>   */
>  function contextChangeTaskProgress(aEvent, aProgress) {
> +    if (gTabmail.currentTabInfo.mode.type == "calendarTask") {

Here maybe exists the same gTabmail problem.

@@ +176,3 @@
>   */
>  function contextChangeTaskPriority(aEvent, aPriority) {
> +    let tabType = gTabmail.currentTabInfo.mode.type;

also here

@@ +204,5 @@
>      if (!duration) {
>          cal.LOG("[calendar-task-tree] Postpone Task - Invalid duration " + aDuration);
>      }
>  
> +    if (gTabmail.currentTabInfo.mode.type == "calendarTask") {

and here again.

Comment 24

[Paul Morris [:pmorris]]

Attachment: events-tasks-menu-items-aug21.patch

This patch addresses issues raised with previous version and passes mozmill tests.

(Also, thanks for the help with fixing the bustage with the previous View > Toolbars menu patch.)

(In reply to [:MakeMyDay] from comment #22)
> Comment on attachment 8780918 [details] [diff] [review]
> events-tasks-menu-items-aug14.patch
> 
> Review of attachment 8780918 [details] [diff] [review]:
> -----------------------------------------------------------------

> 
> @@ +205,5 @@
> >          cal.LOG("[calendar-task-tree] Postpone Task - Invalid duration " + aDuration);
> >      }
> >  
> > +    if (gTabmail.currentTabInfo.mode.type == "calendarTask") {
> > +        postponeTask(aDuration);
> 
> why don't you pass duration here? This would save a type conversion later on
> in postponeTask()

I tried this but the duration object was not serializable for message passing with the iframe, so I kept it as is, passing a string and then converting it to a duration once we're inside the iframe.

> @@ +2702,5 @@
> >    // back to COMPLETED). When we go to store this VTODO as .ics the
> >    // date will get lost.
> >  
> >    // remember the original values
> >    let oldPercentComplete = getElementValue("percent-complete-textbox");
> 
> can you check whether you can make this
> parseInt(getElementValue("percent-complete-textbox")) and get rid of making
> the numeric value a string subsequently?

This works so I changed everything to use numbers instead of strings.

> @@ +2719,4 @@
> >        }
> >    }
> >  
> > +  let completedDate = aPassedInCompletedDate ? aPassedInCompletedDate : null;
> 
> You simply can assign a default value of null above and then use the
> argument directly subsequently:
> 
> function updateToDoStatus(aStatus, aCompletedDate=null) {

Nice!  New ECMAScript features for the win.

> ::: calendar/lightning/content/lightning-item-panel.js
> @@ +496,4 @@
> >   */
> >  function editPriority(aTarget) {
> > +    let newPriority = typeof aTarget == "number" ?
> > +        aTarget : parseInt(aTarget.getAttribute("value"));
> 
> Can you please avoid this type mixture? Either make this two functions, one
> consuming a node object and a second consuming the short, which then can be
> used by the first and also directly from within contextChangeTaskPriority,
> or change all of the other consumers to provide a numeric argument instead
> of a node object.

I made two functions, seemed simpler since the consumers are event handlers in xul files.

(In reply to [:MakeMyDay] from comment #23)
> Comment on attachment 8780918 [details] [diff] [review]
> events-tasks-menu-items-aug14.patch
> 
> Review of attachment 8780918 [details] [diff] [review]:
> -----------------------------------------------------------------

> ::: calendar/base/content/calendar-menus.xml
> @@ +43,4 @@
> >                  }
> >              }
> > +
> > +            if (gTabmail.currentTabInfo.mode.type == "calendarTask") {
> 
> gTabmail doesn't seem to be defined in each context, which prevents the
> menue to be displayed correctly and probably triggers the mozmill test
> failure. Maybe it's enough to add an additional check on whether gTabmail
> exists, but it would be better to understand why it's not available
> sometimes.

I found that making the revisions you recommended already fixed the mozmill tests, but there was still the occasional error about gTabmail not defined.  This seems to be when no event/task tab has been opened yet since TB has started up.  So I added the check that gTabmail is defined and this worked.

Comment 25

[[:MakeMyDay]]

Comment on attachment 8783334 [details] [diff] [review]
events-tasks-menu-items-aug21.patch

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

Thanks, just some nits this time. r=me with that fixed.

::: calendar/base/content/calendar-menus.xml
@@ +43,3 @@
>                  }
>              }
> +            let propertyValue = null;

You can just use let propertyValue; here.

::: calendar/base/content/calendar-task-tree.js
@@ +191,5 @@
>  
>  /**
> + * Handler function to postpone the start and due dates of the selected
> + * tasks, or of the task loaded in the current tab. ISO 8601 format:
> + * "PT1H", "P1D", and "P1W" are 1 hour, 1 day, and 1 week.

Please add a note here that this format is used intentionally instead of an calIDuration object to avoid the serialization problem.

::: calendar/lightning/content/lightning-item-iframe.js
@@ +3019,5 @@
>  
>  /**
> + * Postpone the task's start date/time and due date/time. ISO 8601
> + * format: "PT1H", "P1D", and "P1W" are 1 hour, 1 day, and 1 week.
> + *

Please a the note mentioned above also here to make clear this should not be converted to a calIDuration object.

::: calendar/lightning/content/lightning-item-panel.js
@@ +521,5 @@
> +
> +/**
> + * Edits the priority.
> + *
> + * @param {number} aNewPriority  The new priority value

short

@@ +707,5 @@
> + * menu based on the percent complete value. (The percent complete menu
> + * items are updated by changeMenuByPropertyName in calendar-menus.xml)
> + *
> + * @param {Object} aArg                  Container
> + * @param {string} aArg.percentComplete  The percent complete value as a string

This is a short now.

@@ +717,5 @@
> +}
> +
> +/**
> + * Postpone the task's start date/time and due date/time. ISO 8601
> + * format: "PT1H", "P1D", and "P1W" are 1 hour, 1 day, and 1 week.

And once agein the note about calIDuration.

Comment 26

[Paul Morris [:pmorris]]

Attachment: events-tasks-menu-items-aug22.patch [checked in - comment 27]

Thanks for the review.  This patch makes those changes and passes all mozmill tests.

Comment 27

[Jorg K (CEST = GMT+2)]

Comment on attachment 8783658 [details] [diff] [review]
events-tasks-menu-items-aug22.patch [checked in - comment 27]

https://hg.mozilla.org/comm-central/rev/aea164f9dc22

I assume you wanted this checked in. Please add keyword "checkin-needed" in the future.
I also assume that the bug is done. If so, please
- mark Resolved/Fixed
- set the Target Milestone
- remove keyword "leave-open".

Comment 28

[[:MakeMyDay]]

Jörg, in general, please don't land calendar patches from others if there's no checkin-needed set. The author/reviewer will set it if it's appropriate and they cannot land themselves. In case of the event-in-a-tab project patches require to landed in specific order to apply corretly. In this specific case, you can leave this checked-in.