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 entries for cloneNode() and importNode() deep parameters #11152

Merged
merged 1 commit into from
Jul 14, 2021

Conversation

foolip
Copy link
Contributor

@foolip foolip commented Jun 18, 2021

These entries represent not the parameter itself, but whether it's
optional and defaulting to false. Since that is the current spec'd
behavior, these entries aren't irrelevant in the "removed for two years"
sense, but if they were inverted to mean "defaults to true" they would
be.

This issue is already documented on both MDN pages:
https://developer.mozilla.org/en-US/docs/Web/API/Node/cloneNode
https://developer.mozilla.org/en-US/docs/Web/API/Document/importNode

@foolip foolip requested a review from ddbeck June 18, 2021 15:36
@github-actions github-actions bot added the data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label Jun 18, 2021
@foolip
Copy link
Contributor Author

foolip commented Jun 18, 2021

@ddbeck removed as requested in #11080. I think this is fine, but it's borderline removing accurate data that in a way we usually don't.

Not sure if the MDN pages should also be purged of this historical tidbit?

@ddbeck
Copy link
Collaborator

ddbeck commented Jun 23, 2021

Sorry if it seemed like I was suggesting something unorthodox. Here's what I was thinking:

The "deep optional" feature could also be represented as a feature (of sorts) where deep is required. That feature has been removed from all browsers, so we could drop it as irrelevant. This was the thing that inspired my comment.

The "deep defaults to false" feature is similar in that we could represent it as "deep defaults to true". This does seem less friendly to wholesale removal because it seems that only one browser had a difference worth mentioning. Perhaps a better way to handle this would be to put a note on the parent feature (for Firefox) noting that, for some versions, it had the wrong default value.

@ddbeck ddbeck removed their request for review July 1, 2021 16:16
@foolip
Copy link
Contributor Author

foolip commented Jul 5, 2021

Capturing this in a note seems OK, but then I'd want to test all ancient browsers since most likely the note applies to more than Firefox.

This would be a useless note to show to anyone today, would a range for just that note be acceptable, even without partial_implementation?

If it were up to me I'd just delete these entries. I'm very confident I could find invocations of APIs that work today that didn't work in ancient times that we don't have notes for, and shouldn't add notes for IMHO.

This tension could also be resolved with a better mechanism for applying notes to a range of releases, and making sure that ancient notes are hidden on MDN.

@sideshowbarker
Copy link
Contributor

This one needs rebasing…

These entries represent not the parameter itself, but whether it's
optional and defaulting to false. Since that is the current spec'd
behavior, these entries aren't irrelevant in the "removed for two years"
sense, but if they were inverted to mean "defaults to true" they would
be.

This issue is already documented on both MDN pages:
https://developer.mozilla.org/en-US/docs/Web/API/Node/cloneNode
https://developer.mozilla.org/en-US/docs/Web/API/Document/importNode
@foolip foolip force-pushed the deeply-irrelevant branch from d013201 to cc231f0 Compare July 14, 2021 10:40
@foolip
Copy link
Contributor Author

foolip commented Jul 14, 2021

I wrote http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=9482 to test the cloneNode() behavior a bit, and it's looking like this was only ever a Firefox problem.

This could be represented as notes for Firefox, but it just seems like a net negative to represent this fact at all in BCD. Unless we have a way of hiding the information everywhere web developers might see it, either a note or a separate partial_implementation range is something a web developer might spend a few seconds looking at, for no good at all, since Firefox 29 was released in 2014.

I'm now much more enthusiastic about deleting :)

@sideshowbarker sideshowbarker merged commit bc6fafd into mdn:main Jul 14, 2021
@foolip foolip deleted the deeply-irrelevant branch July 14, 2021 11:52
@ddbeck ddbeck added needs content update This PR needs a corresponding update to mdn/content to update the documentation and removed rebase needed 🚧 labels Jul 15, 2021
@ddbeck
Copy link
Collaborator

ddbeck commented Jul 15, 2021

@foolip @sideshowbarker This got merged too quickly, unfortunately. If we're going to remove these features completely, then content changes are needed on both pages (which mention these historic oddities).

ddbeck added a commit to ddbeck/browser-compat-data that referenced this pull request Jul 15, 2021
ddbeck added a commit that referenced this pull request Jul 15, 2021
* Bump version to v3.3.11

* Add release note for #11502

* Add release note for #11499

* Add release note for #11100

* Add release note for #11101

* Add release note for #11152

* Add release note for #11355

* Add release note for #11361

* Add release note for #11438

* Add release note for #11520

* Add release note for #11452

* Add release note for #11481

* Add second release note for #11481

* Add release note for #11500

* Add release note for #11524

* Add release note for #11413

* Add release note for #11414

* Add release date

* Add stats

* Fix formatting
foolip added a commit to foolip/content that referenced this pull request Jul 15, 2021
@foolip
Copy link
Contributor Author

foolip commented Jul 15, 2021

@ddbeck I've sent mdn/content#6934 to also remove the notes from content.

@ddbeck ddbeck removed the needs content update This PR needs a corresponding update to mdn/content to update the documentation label Jul 15, 2021
sideshowbarker pushed a commit to mdn/content that referenced this pull request Jul 15, 2021
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.

4 participants