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

menus.create.accessKey - Probably wrong data, Firefox does not support this parameter/property #23521

Open
jobisoft opened this issue Jun 26, 2024 · 5 comments
Labels
data:webext 🎲 Compat data for Browser Extensions. https://developer.mozilla.org/Add-ons/WebExtensions

Comments

@jobisoft
Copy link
Contributor

What type of issue is this?

Incorrect support data (example: BrowserX says "86" but support was added in "40")

What information was incorrect, unhelpful, or incomplete?

It is listed here as beeing supported since 63:

"version_added": "63"

The Firefox schema file does not know that parameter:
https://searchfox.org/mozilla-central/rev/2f48061aef8c8976b73749ee845e7b85751f5f2f/browser/components/extensions/schemas/menus.json#193-314

What browsers does this problem apply to, if applicable?

Firefox

What did you expect to see?

"firefox": {
   "version_added": false
 },

Notes:

  • The entire entry for menus.create seems to be not conforming to the official suggestions
  • There is currently no indication in BCD that the parameter accessKey (together with all the other parameters) is actually a property of the createProperties parameter. The linked document suggest to use createProperties_accessKey_parameter.
  • Other places in BCD use simple nested features
menus: {
 createProperties: {
   accessKey : {
   ...
   }
 }
}
  • I do prefer this notation over the suggested notation, since it is easier for clients to access and map the data to the actual schema (linter projects or typeScript definition generator projects).
  • It could be, that the Chrome also does not support this parameter. I have no knowledge if this parameter is at all specified. Where could this be checked?

Did you test this? If so, how?

No. Comparing with the schema file should be sufficient.

Can you link to any release notes, bugs, pull requests, or MDN pages related to this?

No response

Do you have anything more you want to share?

No response

MDN URL

No response

MDN metadata

No response

@Rob--W
Copy link
Member

Rob--W commented Jun 26, 2024

The "access key" feature means recognizing & in the title option and interpreting that as an access key: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/menus/create#title

To reduce confusion it may make sense to move it under the title key in the schema data.

In general the BCD can list features that aren't API properties but references to special behavior. There are no established patterns for documenting that.

@Rob--W Rob--W added the data:webext 🎲 Compat data for Browser Extensions. https://developer.mozilla.org/Add-ons/WebExtensions label Jun 26, 2024
@jobisoft
Copy link
Contributor Author

I offer to work on this, which will probably help me to get a deeper understanding of this matter.

If I am allowed to contribute, may I:

  1. Fix the notation of the properties in the same patch?
  2. Use the nested notation as seen here?

@Rob--W
Copy link
Member

Rob--W commented Jun 30, 2024

What do you mean by fixing the notation of the properties?

Previously I wrote "To reduce confusion it may make sense to move it under the title key in the schema data.", under the assumption that the "title" property was already listed in the BCD. Since it is not there, there is no separate "title" entry in the BCD and suggesting to move it there is not very meaningful.

The BCD already lists a separate description,

"accessKey": {
"__compat": {
"description": "<code>&amp;</code> in <code>title</code> sets access key",

... so the "accessKey" property name does not appear anywhere. Renaming it to something else doesn't add much value at this point.

2. Use the nested notation as seen here?

That was what I was thinking. In this specific case, it is not rendered in the BCD because the intermediate entry is missing the __compat key. I wish that we handled that better... At the least we could render items with compat data, and omit the compat info from intermediate rows (to signal that the compat is unknown).
And it would be nice if we have a unified convention for declaring methods vs properties/parameters (or marking something as a top-level method). Once we have that we could update the {{Compat}} macro to render the BCD with the right name (instead of the current practice which joins the key names together with a dot as separator).

@jobisoft
Copy link
Contributor Author

jobisoft commented Jul 1, 2024

My idea was to use the suggested notation for properties here, and thus do the following renaming:

  • command -> createProperties_command_parameter
  • icons -> createProperties_icons_parameter
  • visible -> createProperties_visible_parameter

I then would add the missing createProperties_title_parameter entry and move the accessKey to become a child of createProperties_title_parameter

My alternative suggestion would be to not use the <paramName>_<propName>_parameter notation, but instead create a new createProperties entry and move all the listed properties as children (without any special name convention), as it is done here. I would then propose to officially add that notation as a suggested notation of properties as well.

The currently used notation makes it difficult for clients to work with BCD, there is no indication that command, icons and visible are properties of the createProperties parameter. The client can check for different notations, but that makes everything very difficult.

@dotproto
Copy link
Collaborator

Might it be best to open an issue about establishing a for documenting browser-specific support for the interpretation of specific fields in objects and documenting that pattern in the BCD Data guidelines?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:webext 🎲 Compat data for Browser Extensions. https://developer.mozilla.org/Add-ons/WebExtensions
Projects
None yet
Development

No branches or pull requests

3 participants