mdn / browser-compat-data

This repository contains compatibility data for Web technologies as displayed on MDN
https://developer.mozilla.org
Creative Commons Zero v1.0 Universal
4.89k stars 1.97k forks source link

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

Open jobisoft opened 2 months ago

jobisoft commented 2 months ago

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: https://github.com/mdn/browser-compat-data/blob/1d41e2a55cdc2addf4b96109948397158d3b7a2b/webextensions/api/menus.json#L552

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:

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 commented 2 months ago

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.

jobisoft commented 2 months ago

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 commented 2 months ago

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,

https://github.com/mdn/browser-compat-data/blob/1d41e2a55cdc2addf4b96109948397158d3b7a2b/webextensions/api/menus.json#L543-L545

... 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 commented 2 months ago

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

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 commented 1 month ago

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?

Rob--W commented 1 month ago

This issue was filed because the accessKey was present in the BCD despite there not being a corresponding API entry. As noted in https://github.com/mdn/browser-compat-data/issues/23521#issuecomment-2198636357, the data itself is not incorrect, so this issue could be closed.

Also noted in the same comment is the issue that many BCD entries are not rendered in the Browser Compatibility table on MDN because of missing __compat keys. I'm not sure if this is an issue in the data itself (i.e. al data should list __compat) or in the macro that renders the compat table. To answer this question, we'd need to agree on what the data should look like.

And in order to have a discussion on the desired format of the BCD data, it would be helpful to describe common patterns in the extension APIs, especially when different from what's usual on the web platform. And also the use cases that we're trying to optimize for (just the "Browser Compatibility" table? or something more?).

Note that the original report here questioned the presence of the accessKey because the property is not present in the extension API. An equally valid question could also have been the question of whether menus.create.accessKey is a method, a property, or whether menus is a method and create.accessKey a property of the option named create.

And even if we end up agreeing on some convention for distinguishing properties from method names, there is also the question on how to account for types. For example, Chrome commonly uses single-use named types to describe objects. These names are not visible in the extension API itself (except when it is an enum), but do appear in their documentation. E.g. contextMenus.CreateProperties (https://developer.chrome.com/docs/extensions/reference/api/contextMenus#type-CreateProperties). Would the name then be contextMenus.create.options.title.accessKey? contextMenus.create.options_parameter.title.accessKey? contextMenus.create.accessKey? contextMenus.CreateProperties.accessKey? etc.