-
Notifications
You must be signed in to change notification settings - Fork 22.5k
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 runtime.on performance warning #32222
Conversation
Preview URLs
Flaws (3)Note! 2 documents with no flaws that don't need to be listed. 🎉 URL:
URL:
URL:
External URLs (1)URL:
(comment last updated: 2024-03-14 03:00:47) |
|
||
- : The function called when this event occurs. The function is passed these arguments: | ||
- `category` | ||
- : `string`. "content_script", the performance warning event category. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be the OnPerformanceWarningCategory
enum. It is also exposed as the runtime.OnPerformanceWarningCategory
object. It currently reports content_script categories only, but the API by design is extensible to other kinds of warnings.
- : `string`. "content_script", the performance warning event category. | ||
- `severity` | ||
- : `string`. The performance warning event severity. One of "low", "medium", or "high". | ||
- `tabId` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tabId
is optional.
files/en-us/mozilla/add-ons/webextensions/api/runtime/onperformancewarning/index.md
Outdated
Show resolved
Hide resolved
|
||
- `listener` | ||
|
||
- : The function called when this event occurs. The function is passed these arguments: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not passed these args but an object with the following properties.
See this example: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/webNavigation/onBeforeNavigate
|
||
{{Compat}} | ||
|
||
> **Note:** This API is based on Chromium's [`chrome.runtime`](https://developer.chrome.com/docs/extensions/reference/runtime/#event-onPerformanceWarning) API. This documentation is derived from [`runtime.json`](https://chromium.googlesource.com/chromium/src/+/master/extensions/common/api/runtime.json) in the Chromium code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please drop this boiletplate. This API doesn't even exist in Chromium.
Co-authored-by: Rob Wu <[email protected]>
|
||
- `category` | ||
- : A {{WebExtAPIRef("runtime.OnPerformanceWarningCategory")}} object indicating the category of the warning. | ||
- `severity` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Rob--W Is this also an object?, e.g. OnPerformanceWarningSeverity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rebloor If it helps, here are the entries from /toolkit/components/extensions/schemas/runtime.json:
{
"id": "OnPerformanceWarningCategory",
"type": "string",
"enum": ["content_script"],
"description": "The performance warning event category, e.g. 'content_script'."
},
{
"id": "OnPerformanceWarningSeverity",
"type": "string",
"enum": ["low", "medium", "high"],
"description": "The performance warning event severity. Will be 'high' for serious and user-visible issues."
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kzar exactly, hence the question.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's inline the potential values for now; I'll clarify with other engineers whether we really want to expose the supported values on an object. If so, then we can update the documentation to add extra articles with the enum values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I requested clarification at w3c/webextensions#456 (comment). Let's wait a week or so to allow for an answer before merging this PR.
I think that the most likely outcome is that we would retain these objects. Let's shape this patch towards that assumption since it is the least amount of work for you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Rob--W I see that @kzar responded to w3c/webextensions#456 (comment) - do I read that correctly that we are agreeing to document OnPerformanceWarningCategory
and OnPerformanceWarningSeverity
as types?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
- : `object`. An object with the following properties: | ||
|
||
- `category` | ||
- : A {{WebExtAPIRef("runtime.OnPerformanceWarningCategory")}} object indicating the category of the warning. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it not an object, but a string that holds one of the enum values.
|
||
- `category` | ||
- : A {{WebExtAPIRef("runtime.OnPerformanceWarningCategory")}} object indicating the category of the warning. | ||
- `severity` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's inline the potential values for now; I'll clarify with other engineers whether we really want to expose the supported values on an object. If so, then we can update the documentation to add extra articles with the enum values.
|
||
Values of this type are strings. Possible values are: | ||
|
||
- `"content_script"`: The performance warning originated from a content script. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- `"content_script"`: The performance warning originated from a content script. | |
- `"content_script"`: The performance warning relates to a slow extension content script. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: avoid ambiguity: the event is for content scripts of the extension itself, not others.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with kzar's suggestion applied.
Description
Adds API documentation and release note for the
runtime.on performance
event introduced by Bug 1861445 Add new hang warning event API for WebExtensionswarning.See mdn/browser-compat-data#22196 and mdn/browser-compat-data#22617 for browser compatibility data.
DO NOT MERGE until related BCD content is ready to merge.