Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add special shortcut _execute_action #22018

Merged
merged 8 commits into from Nov 3, 2022
Merged

Conversation

rebloor
Copy link
Contributor

@rebloor rebloor commented Nov 2, 2022

Description

Adds details about the special shortcut _execute_action which became available to MV3 Extensions with the introduction of the action API and manifest key.

Related issues and pull requests

Addresses the outstanding documentation needs of Bug 1706398 Implement "action" API / manifest key.

@rebloor rebloor added the Content:WebExt WebExtensions docs label Nov 2, 2022
@rebloor rebloor requested a review from Rob--W November 2, 2022 23:05
@rebloor rebloor self-assigned this Nov 2, 2022
@rebloor rebloor requested a review from a team as a code owner November 2, 2022 23:05
@rebloor rebloor requested review from willdurand and removed request for a team November 2, 2022 23:05
@github-actions github-actions bot added the Content:Other Any docs not covered by another "Content:" label label Nov 2, 2022
@rebloor rebloor requested a review from a team as a code owner November 2, 2022 23:16
@rebloor rebloor requested review from bsmth and removed request for a team November 2, 2022 23:16
@@ -22,9 +22,12 @@ An `object` passed to the {{WebExtAPIRef("menus.create()", "menus.create()")}} o
- : `string`. String describing an action that should be taken when the user clicks the item. Possible values are:

- `"_execute_browser_action"`: simulate a click on the extension's browser action, opening its popup if it has one
- `"_execute_action"`: simulate a click on the extension's action, opening its popup if it has one
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Rob--W This file appears to be orphaned. menu.create includes the information directly and menu.update makes reference to menu.create. Should I delete this page (with an appropriate redirect)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I seem to have an issue with the delete:

yarn content delete Mozilla/Add-ons/WebExtensions/API/menus/createProperties
yarn run v1.22.18
$ env-cmd --silent cross-env CONTENT_ROOT=files yari-tool delete Mozilla/Add-ons/WebExtensions/API/menus/createProperties
node:events:505
      throw er; // Unhandled 'error' event
      ^

Error: spawn yari-tool ENOENT
    at Process.ChildProcess._handle.onexit (node:internal/child_process:283:19)
    at onErrorNT (node:internal/child_process:478:16)
    at processTicksAndRejections (node:internal/process/task_queues:83:21)
Emitted 'error' event on ChildProcess instance at:
    at Process.ChildProcess._handle.onexit (node:internal/child_process:289:12)
    at onErrorNT (node:internal/child_process:478:16)
    at processTicksAndRejections (node:internal/process/task_queues:83:21) {
  errno: -2,
  code: 'ENOENT',
  syscall: 'spawn yari-tool',
  path: 'yari-tool',
  spawnargs: [
    'delete',
    'Mozilla/Add-ons/WebExtensions/API/menus/createProperties'
  ]
}
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Thoughts? Open a ticket on the yari?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you install the dependencies? The error seems to suggest that yari-tool is not available.

According to #16945, the binaries are provided by @mdn/yari.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assumed so (as I've been running yarn start for a while). However, reinstalling has resolved the issue. Thanks.

@@ -44,9 +44,12 @@ browser.menus.create(
- : `string`. String describing an action that should be taken when the user clicks the item. Possible values are:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Rob--W was your feedback to remove the list from here and simply rely on a link to the special shortcut documentation?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main point was to link a central place with detailed documentation, which you have done. Deletion of the list is not strictly necessary.

One change that may be considered is the addition of "(MV2)" to _execute_browser_action and "(MV3)" to _execute_action (across all 3 pages where this concise list appears).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

- `"_execute_page_action"`: simulate a click on the extension's page action, opening its popup if it has one
- `"_execute_sidebar_action"`: open the extension's sidebar

See the documentation of special shortcuts in the manifest.json key [`commands`](/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/commands#special_shortcuts) for details.

Clicking the item will still trigger the {{WebExtAPIRef("menus.onClicked")}} event, but there's no guarantee of the ordering here: the command may be executed before `onClicked` fires.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This documentation is incorrect. When the command is supported, the onClicked event is not fired. When it's not recognized, the event is fired, which the extension can use to implement a fallback if they like.

This behavior changed in Fx 91 in the bug associated with this PR.

This review comment applies to menus.create and update.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm slightly confused by the use of the term supported and recognized. Do you mean whether the special shortcut is implemented or not implemented? I've attempted an update, please confirm.

@@ -22,9 +22,12 @@ An `object` passed to the {{WebExtAPIRef("menus.create()", "menus.create()")}} o
- : `string`. String describing an action that should be taken when the user clicks the item. Possible values are:

- `"_execute_browser_action"`: simulate a click on the extension's browser action, opening its popup if it has one
- `"_execute_action"`: simulate a click on the extension's action, opening its popup if it has one
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

<tr>
<th></th>
<th>Manifest V2</th>
<th>Manifiest V3</th>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<th>Manifiest V3</th>
<th>Manifest V3</th>

Typo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@@ -16,7 +16,7 @@ A popup is a dialog that's associated with a [toolbar button](/en-US/docs/Mozill

When the user clicks the button, the popup is shown. When the user clicks anywhere outside the popup, the popup is closed. The popup can be closed programmatically by calling [`window.close()`](/en-US/docs/Web/API/Window/close) from a script running in the popup. However, you can't open the popup programmatically from an extension's JavaScript; it can be opened only in response to a user action.

You can define a keyboard shortcut that opens the popup using the `"_execute_browser_action"` and `"_execute_page_action"` shortcuts. See the documentation for the manifest.json key [`commands`](/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/commands).
You can define a keyboard shortcut that opens the popup using the `"_execute_browser_action"` and `"_execute_page_action"` shortcuts in Manifest V2 and `"_execute_action"`and, where supported, `"_execute_page_action"` shortcuts in Manifiest V3. See the documentation for the special shortcuts in manifest.json key [`commands`](/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/commands#special_shortcuts).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
You can define a keyboard shortcut that opens the popup using the `"_execute_browser_action"` and `"_execute_page_action"` shortcuts in Manifest V2 and `"_execute_action"`and, where supported, `"_execute_page_action"` shortcuts in Manifiest V3. See the documentation for the special shortcuts in manifest.json key [`commands`](/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/commands#special_shortcuts).
You can define a keyboard shortcut that opens the popup using the `"_execute_browser_action"` and `"_execute_page_action"` shortcuts in Manifest V2 and `"_execute_action"`and, where supported, `"_execute_page_action"` shortcuts in Manifest V3. See the documentation for the special shortcuts in manifest.json key [`commands`](/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/commands#special_shortcuts).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@@ -44,9 +44,12 @@ browser.menus.create(
- : `string`. String describing an action that should be taken when the user clicks the item. Possible values are:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main point was to link a central place with detailed documentation, which you have done. Deletion of the list is not strictly necessary.

One change that may be considered is the addition of "(MV2)" to _execute_browser_action and "(MV3)" to _execute_action (across all 3 pages where this concise list appears).

@rebloor rebloor requested a review from Rob--W November 3, 2022 16:15

Clicking the item will still trigger the {{WebExtAPIRef("menus.onClicked")}} event, but there's no guarantee of the ordering here: the command may be executed before `onClicked` fires.
When specified, clicking the item does not trigger the {{WebExtAPIRef("menus.onClicked")}} event; instead, the pop-up is opened. When not specified, clicking the item triggers {{WebExtAPIRef("menus.onClicked")}} and can be used to implement fallback behavior.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
When specified, clicking the item does not trigger the {{WebExtAPIRef("menus.onClicked")}} event; instead, the pop-up is opened. When not specified, clicking the item triggers {{WebExtAPIRef("menus.onClicked")}} and can be used to implement fallback behavior.
When the given command is recognized by the browser, clicking the item does not trigger the {{WebExtAPIRef("menus.onClicked")}} event; instead, the associated action is triggered, such as opening the pop-up. When not specified or not recognized, clicking the item triggers {{WebExtAPIRef("menus.onClicked")}} and can be used to implement fallback behavior.

Suggested change of message; rephrase as needed.

The issue with the previous wording was that it seemed to suggest that if someone were to specify command: "bogus", that the onClicked listener would not be triggered. That's inaccurate. The listener is only not triggered if the command is recognized.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check my update

@willdurand willdurand removed their request for review November 3, 2022 19:58
@rebloor rebloor requested a review from Rob--W November 3, 2022 21:12
@rebloor rebloor merged commit c8ea5e9 into mdn:main Nov 3, 2022
@rebloor rebloor deleted the action-aliases branch November 4, 2022 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:Other Any docs not covered by another "Content:" label Content:WebExt WebExtensions docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants