1338727 - support key commands for sidebars

Status: RESOLVED FIXED

(WebExtensions :: Frontend, defect, P2)

Description

[Shane Caraveo (:mixedpuppy)]

ext-commands.js should support _execute_sidebar_action to support key bindings for a sidebar.

Comment 1

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1338727 add command support for sidebar-action,

Review commit: https://reviewboard.mozilla.org/r/112154/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/112154/

Comment 2

[Andrew Swan [:aswan]]

Comment on attachment 8836842 [details]
Bug 1338727 add command support for sidebar-action,

https://reviewboard.mozilla.org/r/112154/#review113902

::: browser/components/extensions/ext-commands.js:136
(Diff revision 1)
>        } else if (name == "_execute_browser_action") {
>          let win = event.target.ownerGlobal;
>          browserActionFor(this.extension).triggerAction(win);
> +      } else if (name == "_execute_sidebar_action") {
> +        let win = event.target.ownerGlobal;
> +        sidebarActionFor(this.extension).triggerAction(win);

Check that you actually get back a value here so this doesn't throw if this gets invoked in an extension that doesn't have a sidebar.
I guess we don't do the same for page action and browser action, that could get fixed too :-)

::: browser/components/extensions/test/browser/browser_ext_commands_execute_sidebar_action.js:45
(Diff revision 1)
> +    },
> +  });
> +
> +  yield extension.startup();
> +  yield SimpleTest.promiseFocus(window);
> +  // Since we didn't set useAddonManager, the sidebar will not be automatically

I don't get this, what does useAddonManager have to do with this?

Comment 3

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8836842 [details]
Bug 1338727 add command support for sidebar-action,

https://reviewboard.mozilla.org/r/112154/#review113902

> Check that you actually get back a value here so this doesn't throw if this gets invoked in an extension that doesn't have a sidebar.
> I guess we don't do the same for page action and browser action, that could get fixed too :-)

This is also a problem with page and browser action.  I'll fix for all.

> I don't get this, what does useAddonManager have to do with this?

The sidebar is only automatically opened during install of the addon.  If you dont have useAddonManager set in the test, you don't get the ADDON_INSTALL reason.

Comment 4

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8836842 [details]
Bug 1338727 add command support for sidebar-action,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/112154/diff/1-2/

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/673306dc657e
add command support for sidebar-action, r=aswan

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/673306dc657e

Comment 7

[Will Bamberg [:wbamberg]]

Updated https://developer.mozilla.org/en-US/Add-ons/WebExtensions/manifest.json/commands#Special_shortcuts

Does this cover it?

Comment 8

[Shane Caraveo (:mixedpuppy)]

Yep, Thanks!