Skip to content

DNR properties changes#23400

Merged
rebloor merged 4 commits intomdn:mainfrom
rebloor:DNR-properties-changes
Jul 13, 2024
Merged

DNR properties changes#23400
rebloor merged 4 commits intomdn:mainfrom
rebloor:DNR-properties-changes

Conversation

@rebloor
Copy link
Copy Markdown
Contributor

@rebloor rebloor commented Jun 16, 2024

Summary

Adds browser compatibility data in support of Bug 1894128
[DNR] Replace MAX_NUMBER_OF_DYNAMIC_AND_SESSION_RULES including:

  • adding a note that MAX_NUMBER_OF_DYNAMIC_AND_SESSION_RULES was deprecated in Firefox 128 (I haven't marked the entire property as deprecated given that Safari hasn't yet made any updates)
  • MAX_NUMBER_OF_DYNAMIC_RULES and MAX_NUMBER_OF_SESSION_RULES - new entries, including documentation links (Chrome release number determined from https://chromium.googlesource.com/chromium/src/+/eab7fc99e59b69e929d02e43bdcf8bbd75333869/chrome/VERSION)
  • MAX_NUMBER_OF_UNSAFE_DYNAMIC_RULES and MAX_NUMBER_OF_UNSAFE_SESSION_RULES - new entries, excluding documentation links as these haven't been been implemented in Firefox yet (to be implemented in bug 1894119)

Related issues

Related changes to MDN content in mdn/content#34214

@rebloor rebloor added the data:webext Compat data for Browser Extensions. https://developer.mozilla.org/Add-ons/WebExtensions label Jun 16, 2024
@rebloor rebloor requested a review from dotproto June 16, 2024 23:40
@rebloor rebloor self-assigned this Jun 16, 2024
@@ -148,7 +148,8 @@
},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I reached out to @oliverdunk about Chrome's support of the old property. It appears that while Chrome did not formally mark this property as deprecated, the intent was to replace it. The fact that nodoc was added to MAX_NUMBER_OF_DYNAMIC_AND_SESSION_RULES in the same CL that removed the nodoc flags from the new properties is indicative of Chrome's intent to deprecate this property.

See also WIP Chromium CL 5637030.

Suggested change
},
"note": "Deprecated in Chrome 120"
},

Copy link
Copy Markdown
Contributor

@dotproto dotproto Jun 18, 2024

Choose a reason for hiding this comment

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

Resolved in commit dae1894.

@rebloor rebloor requested a review from dotproto June 18, 2024 04:14
@rebloor rebloor marked this pull request as ready for review July 12, 2024 17:34
@rebloor rebloor merged commit 5e0da9d into mdn:main Jul 13, 2024
@rebloor rebloor deleted the DNR-properties-changes branch July 13, 2024 17:43
@queengooborg
Copy link
Copy Markdown
Contributor

queengooborg commented Jul 13, 2024

Hi @rebloor -- this PR should not have been merged yet if there was a failing test, as BCD requires CI tests to pass before merging.

I removed the MDN URLs that were causing the CI to fail in #23793. The new MDN URL linter should automatically add these back once the mdn-content-inventory has been updated and includes the information about the new pages.

@rebloor
Copy link
Copy Markdown
Contributor Author

rebloor commented Jul 15, 2024

@queengooborg I wasn't aware of the linter behavior regarding the addition of URLs once documentation is published. Where would I have found information about this new behavior? So, from now on, for new docs, I shouldn't include the "mdn_url" but wait for them to be added automatically.

@queengooborg
Copy link
Copy Markdown
Contributor

Sorry for not responding to this earlier, this got buried in my notifications.

New linter behavior isn't something that we've documented in BCD, as it is an internal change that impacts contributors, not consumers. (Actually, hardly any specific linter rules are documented; only the ones that were originally non-linted policies are documented.) New linter rules are, however, almost always discussed during the BCD weekly calls and aren't implemented before they receive owner consensus.

The motivation for the new MDN URL linter is to reduce the need for manual updates to MDN URLs when pages are created, moved or deleted. So far, it's been mostly helpful, though this is a case where it hasn't been quite so helpful (the pages were updated but the linter didn't have the updated information yet -- which is why I brought up the idea of making the URL linter warn instead of error).


The important part is just making sure that the CI always returns good before merging a PR, even if the issue will eventually fix itself when dependencies are updated. This is because unlike MDN content, the CI tests for PRs run on EVERY file, not just the ones that are modified. Thus, if a PR with a failing test is merged, all other open PRs suddenly have failing tests, which halts all BCD development until the CI passes again!

@rebloor
Copy link
Copy Markdown
Contributor Author

rebloor commented Aug 1, 2024

@queengooborg thanks for that, should I come across another similar issue I'll check with you first rather than assuming it's an issue that would be otherwise fixed. (Are there minutes from the weekly call? Something I could search if I do come across the situations.)

@queengooborg
Copy link
Copy Markdown
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

data:webext Compat data for Browser Extensions. https://developer.mozilla.org/Add-ons/WebExtensions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants