Conversation
|
Tip: Review these changes grouped by change (recommended for most PRs), or grouped by feature (for large PRs). |
webextensions/api/webNavigation.json
Outdated
| "__compat": { | ||
| "support": { | ||
| "chrome": { | ||
| "version_added": "106" |
There was a problem hiding this comment.
Is this value correct? Chrome's API documentation indicates that this property was added in Chrome 74. https://developer.chrome.com/docs/extensions/reference/api/webNavigation#property-onCommitted-callback-details-parentFrameId
It also looks like this property is exposed on nine Web Navigation API events in total.
There was a problem hiding this comment.
That's annoying, because I knew that (as you see, I even commented on it in the summary information for the PR). Exposure in the other events, I did overlook. However, only introduced in 74 in onCompleted, onDOMContentLoaded, onErrorOccurred, onHistoryStateUpdated, and onReferenceFragmentUpdated, and those are now added.
There was a problem hiding this comment.
Looks like Chrome added a number of other Chrome-only properties to various events and methods in the same commit in 106:
- Methods
getFrame- arguments
details.documentId
- return value
details.documentIddetails.parentDocumentIddetails.documentLifecycledetails.frameType
- arguments
getAllFrames- return value
details[].documentIddetails[].parentDocumentIddetails[].documentLifecycledetails[].frameType
- return value
- Events
onBeforeNavigatehasparentDocumentId,documentLifecycle, andframeType- the following events all have
documentId,parentDocumentId,documentLifecycle, andframeTypeonDOMContentLoadedonCompletedonErrorOccurredonReferenceFragmentUpdatedonHistoryStateUpdated
Additionally, several of the events on this namespace don't have information for any of their properties. I don't want to blow out the scope this PR by trying to tackle everything here, so we should create an issue to document all of the event properties on this namespace.
@rebloor, would you prefer to address the Chrome-only properties as part of this PR or as part of the "document all event properties" task? I'm good with either approach.
There was a problem hiding this comment.
@dotproto done, with the corresponding documentation updates in mdn/content#39893
There was a problem hiding this comment.
Looks like the getAllFrames() function and onReferenceFragmentUpdated event aren't included. Was that intentional?
There was a problem hiding this comment.
@dotproto The change is for getAllFrames() are from line 322 and onReferenceFragmentUpdated from line 1433. I double-checked everything else, and it all appears to be there.
There was a problem hiding this comment.
My mistake. Thanks for providing the additional details.
…hrome-only-properties
|
@rebloor This fell through the cracks. Could you update the PR please? This is part of the work needed for mdn/content#39630 |
There was a problem hiding this comment.
My mistake. Thanks for providing the additional details.
Rob--W
left a comment
There was a problem hiding this comment.
NOTE: this is not blocking the merge, feel free to merge what you have already, and create a new PR with a follow-up.
I started offering suggested edits, but stopped when I noticed that some part of the diff cannot be edited because of the way that Github hides "unmodified" lines.
Could you update all entries of documentId and parentDocumentId and set Safari's support to 18.4? It is part of https://developer.apple.com/documentation/safari-release-notes/safari-18_4-release-notes#Web-Extensions
(there are many other documentId properties that have yet to be documented on MDN and BCD - see w3c/webextensions#890 (comment) and comments below that )
Co-authored-by: Rob Wu <rob@robwu.nl>
|
@Rob--W I completed the relevant support information in this PR and created Safari 18.4 support for documentId #28341 to cover changes in other APIs. |
Summary
Add details of properties added to
webNavigation.onCommittedin Chrome in version 74 and 106.Related issues
Addresses the issue raised in webNavigation.onCommitted details does not mention discrepancy with Chromium #39630 by providing clarification that these properties are only supported in Chrome.
MDN content update in mdn/content#39893