Closed Bug 1300845 Opened 3 years ago Closed 3 years ago

Keyboard shortcuts to create new event or task no longer work [ReferenceError: openNewEvent is not defined]

Categories

(Calendar :: Calendar Views, defect)

Lightning 5.3
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ssitter, Assigned: pmorris)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

Thunderbird 51.0a1 (20160905030200) with built-in Lightning and fresh profile

Steps to reproduce:
Switch to calendar view
Press keyboard shortcut (Ctrl+I) to create new event
Press keyboard shortcut (Ctrl+D) to create new task

Actual result:
Nothing happens. Error Console shows:

ReferenceError: openNewEvent is not defined
ReferenceError: openNewTask is not defined
Blocks: 1277972
Blocks: ltn54
Whiteboard: [l10n impact]
This has no l10n impact, but the functions are not available in the calling context. Paul, can you take a look at this, too? This is a regression of the event-in-a-tab implemnetation.
Flags: needinfo?(paul)
Whiteboard: [l10n impact]
Assignee: nobody → paul
Flags: needinfo?(paul)
For the dialog window case this patch just adds functions to send messages to the iframe's contents to call the existing functions there.  

While testing I discovered the new message and new address book menu commands in the dialog window were also not working.  This patch fixes them by moving the relevant functions out of the iframe.

For the tab case some unneeded commands in lightning-item-panel.xul were overriding the keyboard shortcuts defined for the new event/task commands in the 'events and tasks' menu, and removing them fixed the problem.  This patch also removes all other unnecessary items from lightning-item-panel.xul to avoid any other potential problems.

The privacy toolbar button's drop-down menu wasn't working due to a trivial bad lookup, and this patch fixes it.

To address in a follow-up bug: 'Page Setup' menu command in dialog window case is not working. (PrintUtils is not defined.)
Attachment #8816730 - Flags: review?(makemyday)
Status: NEW → ASSIGNED
Comment on attachment 8816730 [details] [diff] [review]
keyboard-shortcuts-new-events-tasks.patch

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

Thanks. r+ with the comments considered. Please also keep in mind bug 581817 when looking at the print commands.

::: calendar/lightning/content/lightning-item-panel.js
@@ +413,5 @@
> +                                           null,
> +                                           Components.interfaces.nsIMsgCompType.New,
> +                                           Components.interfaces.nsIMsgCompFormat.Default,
> +                                           null,
> +                                           null);

Can you make this

MailServices.compose.OpenComposeWindow(
    null,
    null,
    null,
    Components.interfaces.nsIMsgCompType.New,
    Components.interfaces.nsIMsgCompFormat.Default,
    null,
    null
);

@@ +423,5 @@
> +function openNewCardDialog() {
> +    window.openDialog(
> +        "chrome://messenger/content/addressbook/abNewCardDialog.xul",
> +        "",
> +        "chrome,modal,resizable=no,centerscreen");

Same here.
Attachment #8816730 - Flags: review?(makemyday) → review+
Paul, any chance you can upload an updated patch with the review comments considered before the upcoming merge day in about one week from now so we get this in shape for 52 beta?
Flags: needinfo?(paul)
This patch addresses the comments on previous one.
Attachment #8816730 - Attachment is obsolete: true
Flags: needinfo?(paul)
Attachment #8827297 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/6347868dcbfaa2346ffe13ed1b98a6810f40859d
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 5.5
Will this be backported to Thunderbird 52?
Comment on attachment 8827297 [details] [diff] [review]
keyboard-shortcuts-new-events-tasks-2.patch

Yes, it should, thanks for checking.
Attachment #8827297 - Flags: approval-calendar-beta?(philipp)
Attachment #8827297 - Flags: approval-calendar-beta?(philipp) → approval-calendar-beta+
Sorry: Beta (TB 52, Calendar 5.4).
You need to log in before you can comment on or make changes to this bug.