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

Remove MutationObserverInit API from BCD #12795

Merged
merged 5 commits into from Oct 27, 2021

Conversation

queengooborg
Copy link
Collaborator

This PR removes MutationObserverInit from BCD. This feature is a dictionary, enum, or WebIDL typedef and should not be included in BCD.

This PR removes `MutationObserverInit` from BCD.  This feature is a dictionary, enum, or WebIDL typedef and should not be included in BCD.
@queengooborg queengooborg added needs-release-note 📰 needs content update 📝 This PR needs a corresponding update to mdn/content to update the documentation labels Oct 11, 2021
@github-actions github-actions bot added the data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label Oct 11, 2021
@queengooborg queengooborg removed the needs content update 📝 This PR needs a corresponding update to mdn/content to update the documentation label Oct 11, 2021
Copy link
Collaborator

@foolip foolip left a comment

Choose a reason for hiding this comment

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

Since the versions here are the same parent feature I don't think we need to add the data for parameters. It's only for capturing some notes, and those notes could be added to the observer() entry instead, if they turn out to be useful.

},
"firefox": {
"version_added": "14",
"notes": "Starting in Firefox 36, <code>attributeOldValue</code> has no default value; previously, its default value was <code>false</code>."
Copy link
Collaborator

Choose a reason for hiding this comment

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

This and the other "Starting in Firefox 36" notes were added in #2299 with no discussion, and I don't think they make sense. A boolean dictionary member, even if it doesn't have a default value in the IDL, still almost certainly has a boolean default value internally in the C++ code. It is possible to handle a missing value differently as a 3rd state, but I doubt that was the case here.

https://bugzilla.mozilla.org/show_bug.cgi?id=973638 is marked as shipped in Firefox 36 and changes default values, so I think that's what this was about. That was matching https://www.w3.org/Bugs/Public/show_bug.cgi?id=23189, which actually seems to be exactly the same change as the "requires attributes: true" note for Edge.

http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=9713 is a test for this and I can confirm the issue in Edge 18 and Firefox 35, while it's fixed in Firefox 36. Can you test this to see if the problem was originally in Chrome and Firefox too? I assume it was.

I'm not sure this is worth a note or a separate partial_implementation entry, however. @Elchi3 WDYT?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for writing a test! I found the same behavior until Chrome 33 and Safari 10.1, so I've updated the notes to reflect this. I've also folded the data into the observe() feature itself, since as you mentioned, the compatibility data is the same.

@@ -225,40 +225,52 @@
"spec_url": "https://dom.spec.whatwg.org/#ref-for-dom-mutationobserver-observe②",
"support": {
"chrome": {
"version_added": "18"
"version_added": "18",
"notes": "Before Chrome 33, <code>attributes: true</code> was required when using <code>attributeFilter</code> or <code>attributeOldValue</code>. If <code>attributes: true</code> is not present, then Chrome throws a syntax error."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"notes": "Before Chrome 33, <code>attributes: true</code> was required when using <code>attributeFilter</code> or <code>attributeOldValue</code>. If <code>attributes: true</code> is not present, then Chrome throws a syntax error."
"notes": "Before Chrome 33, <code>attributes: true</code> is required when using <code>attributeFilter</code> or <code>attributeOldValue</code>. If <code>attributes: true</code> is not present, then Chrome throws a syntax error."

@foolip foolip merged commit 86ab7b0 into mdn:main Oct 27, 2021
@queengooborg queengooborg deleted the api/MutationObserverInit/removal branch October 27, 2021 08:08
jdatapple pushed a commit to jdatapple/browser-compat-data that referenced this pull request Oct 27, 2021
This PR removes `MutationObserverInit` from BCD.  This feature is a dictionary, enum, or WebIDL typedef and should not be included in BCD.

Update notes for attributeFilter parameter based upon testing.
ddbeck added a commit to ddbeck/browser-compat-data that referenced this pull request Nov 2, 2021
ddbeck added a commit that referenced this pull request Nov 2, 2021
* Bump version to v4.0.9

* Set release date

* Add release note for #12795

* Add release note for #13006

* Add release note for #13136

* Add release note for #13076

* Add release note for #13216

* Add release note for #11924

* Add release note for #13143

* Undo list of one item

* Hoist known issues

* Add stats

* Fix compare link
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants