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

Set isUrlFilterCaseSensitive to 'false' by default #26908

Merged
merged 4 commits into from Jul 12, 2023
Merged

Set isUrlFilterCaseSensitive to 'false' by default #26908

merged 4 commits into from Jul 12, 2023

Conversation

maximtop
Copy link
Contributor

@maximtop maximtop commented May 22, 2023

w3c/webextensions#269 (comment)

Description

I've updated true to false.

Motivation

To correct documentation

Additional details

w3c/webextensions#269 (comment)

Related issues and pull requests

Fixes #27333

@maximtop maximtop requested a review from a team as a code owner May 22, 2023 16:53
@maximtop maximtop requested review from jpmedley and removed request for a team May 22, 2023 16:53
@github-actions github-actions bot added Content:Other Any docs not covered by another "Content:" label Content:WebExt WebExtensions docs labels May 22, 2023
@jpmedley
Copy link
Collaborator

@maximtop Thank you for submitting this. Can you also please add notes to the browser compat data saying in which versions of each browser the change occurred. A typical wording is something like, "Before Chrome xxx, the default was true."

Let me know if you need any help.

@maximtop
Copy link
Contributor Author

Nope, I do not have such information, I think that it just wasn't fixed in Chrome yet.

@jpmedley
Copy link
Collaborator

@oliverdunk, do you know any of the implementation versions for this change, or at least who to ask for that information?

@github-actions
Copy link
Contributor

github-actions bot commented May 24, 2023

Preview URLs

Flaws (1)

URL: /en-US/docs/Mozilla/Add-ons/WebExtensions/API/declarativeNetRequest/RuleCondition
Title: declarativeNetRequest.RuleCondition
Flaw count: 1

  • broken_links:
    • Can't resolve /docs/Web/API/URL
External URLs (1)

URL: /en-US/docs/Mozilla/Add-ons/WebExtensions/API/declarativeNetRequest/RuleCondition
Title: declarativeNetRequest.RuleCondition

(comment last updated: 2023-06-09 09:35:01)

@oliverdunk
Copy link
Contributor

@oliverdunk, do you know any of the implementation versions for this change, or at least who to ask for that information?

This hasn't been implemented yet. We're blocked on doing some outreach to developers first to let them know the change is coming - I'll try to draft something soon.

@maximtop, maybe worth making it more explicit that the change hasn't happened in Chrome yet?

@maximtop
Copy link
Contributor Author

@oliverdunk @jpmedley
I've updated the description, now it should be clearer.

@oliverdunk
Copy link
Contributor

@oliverdunk @jpmedley I've updated the description, now it should be clearer.

Left one suggestion, but I think that's better :)

Co-authored-by: Oliver Dunk <oliver@oliverdunk.com>
@maximtop
Copy link
Contributor Author

@oliverdunk @jpmedley I've updated the description, now it should be clearer.

Left one suggestion, but I think that's better :)

Thank you

@jpmedley
Copy link
Collaborator

I apologize for not being clear enough about what I'm asking for. These are MDN's requirements, not my personal preferences. I'll explain the reasons behind these requests.

  • Information about specific versions of browsers should be in the browser compat data. This so that the data is available to the many BCD clients. This expressly supports developers attaining interoperability with as little friction as possible.
  • This page should say something like, "Defaults vary by browser and version. See the compatibility table for details." This is so that we never need to touch this text for a very long, and possibly never again. MDN contains pages for some 15,000 interfaces, constructors, methods, properties, etc. That doesn't even included HTML, CSS, or the JavaScript language itself. Avoiding duplication is critical to keeping this massive set of information up to date.
  • In neither location should future plans be discussed. MDN and BCD are not places to advertise what browsers will or might do in the future. They need to focus on what browsers can do now.

@maximtop
Copy link
Contributor Author

Hi @jpmedley,

Thank you for your detailed explanation of MDN's content requirements. I appreciate your insights on the importance of specific browser version details and the non-speculative nature of the content.

However, I wanted to clarify that my primary intention was to fix a bug in the existing documentation.

As I mentioned earlier, I, unfortunately, don't have the information about specific browser versions where the change occurred.

Under the circumstances, I hope you will consider the importance of fixing this bug. If you feel the changes are still beneficial without the specific browser version details, could you consider approving this PR? I believe it could still make a positive contribution to the documentation.

@jpmedley
Copy link
Collaborator

@oliverdunk and I are trying to get this into BCD. Please make the text change I requested to this doc and hang on a bit.

Thank you.

Copy link
Member

@Rob--W Rob--W left a comment

Choose a reason for hiding this comment

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

However, I wanted to clarify that my primary intention was to fix a bug in the existing documentation.

Oops. The "true" should be "false" and that is indeed a high-prio inaccuracy to fix. I don't mind the exact mentions of the BCD as much; these can be edited in parallel or in a follow-up.

As I mentioned earlier, I, unfortunately, don't have the information about specific browser versions where the change occurred.

As for version info for the BCD:

@maximtop
Copy link
Contributor Author

maximtop commented Jun 8, 2023

Thanks, @Rob--W

Chrome no version yet (open bug at https://bugs.chromium.org/p/chromium/issues/detail?id=1414441)

Since this field is already available in Chrome, it would be correct to specify the Chrome version in the BCD, but with a note that its default value will change in the future. However, I don't know which version it appeared in Chrome as there is no version specified in the Chrome documentation. I can infer that it appeared in Chrome since the DNR appeared, but I am not sure if that would be correct.

@Rob--W
Copy link
Member

Rob--W commented Jun 8, 2023

Thanks, @Rob--W

Chrome no version yet (open bug at https://bugs.chromium.org/p/chromium/issues/detail?id=1414441)

Since this field is already available in Chrome, it would be correct to specify the Chrome version in the BCD, but with a note that its default value will change in the future. However, I don't know which version it appeared in Chrome as there is no version specified in the Chrome documentation. I can infer that it appeared in Chrome since the DNR appeared, but I am not sure if that would be correct.

Steps to figure out the answer:

  1. Look up the implementation, cs.chromium.org/isUrlFilterC%20file:idl
  2. Use blame to find the earliest version where the line was introduced. Here is the commit: https://chromium.googlesource.com/chromium/src.git/+/2f01862f0e777dbcf9b224067dee1b5ab9b7193f
  3. In the commit, look for chrome/VERSION.txt in the tree to find the version: in this case 62 - https://chromium.googlesource.com/chromium/src.git/+/2f01862f0e777dbcf9b224067dee1b5ab9b7193f/chrome/VERSION

Now this gives the earliest possible version that has the API definition. That doesn't mean that it's fully implemented. The actual implementation may be incomplete, or the feature could be restricted to a pre-release channel, or restricted to specific extensions, etc. While it's possible to get the exact version by looking up other files, in this case my hunch is that the implementation was completed before the release of the API, so you can rely on the release date documented for the existing RuleConditon entry in the BCD.

@Rumyra
Copy link
Collaborator

Rumyra commented Jun 9, 2023

Hey folks!

I just want to re-iterate something @jpmedley mentioned. Our guidelines are to keep browser specifics out of content and within the browser compatibility data - this makes it much easier for maintenance moving forward (as browser implementations change we don't have to remember where we mentioned it in the docs)

@Rob--W if you're confident the content will be updated once Chrome are complient that's ok, but it might save work in the future if it was just a note in bcd

@Rob--W
Copy link
Member

Rob--W commented Jun 9, 2023

Hey folks!

I just want to re-iterate something @jpmedley mentioned. Our guidelines are to keep browser specifics out of content and within the browser compatibility data - this makes it much easier for maintenance moving forward (as browser implementations change we don't have to remember where we mentioned it in the docs)

In my understanding, the thread above about specific browser versions is to decide on what to put in the BCD.

@Rob--W if you're confident the content will be updated once Chrome are complient that's ok, but it might save work in the future if it was just a note in bcd

I believe that it is useful to call out that the default has not always been the same, while emphasizing that the default converges towards the same value. I believe that the wording I suggested before is an appropriate balance between informative and maintainable. I'm confident that we can make the words even more firm (emphasizing the future "common" default) once the versions have shipped for a while.

@maximtop
Copy link
Contributor Author

Thanks @Rob--W for teaching me how to check version, I thought that way too, and I wasn't sure like you about exact version.

Here @oliverdunk created topic if it would be changed to false https://groups.google.com/a/chromium.org/g/chromium-extensions/c/TXYUmvDHUlw/m/9JdxXG2cAgAJ.

I've added BCD in this PR mdn/browser-compat-data#20130

@Josh-Cena Josh-Cena changed the title It is agreed to set isUrlFilterCaseSensitive to 'false' by default Set isUrlFilterCaseSensitive to 'false' by default Jun 17, 2023
…quest/rulecondition/index.md

Co-authored-by: Rob Wu <rob@robwu.nl>
@github-actions github-actions bot removed the Content:Other Any docs not covered by another "Content:" label label Jul 4, 2023
@maximtop
Copy link
Contributor Author

maximtop commented Jul 4, 2023

@jpmedley I've updated the bcd, and applied suggestion from @Rob--W
Could you please review this pr again?

@Rob--W
Copy link
Member

Rob--W commented Jul 12, 2023

I'll merge this PR since it corrects a factual error (on the consensus). If there is a need to word-smith more, a new PR can be submitted.

@Rob--W Rob--W merged commit b9ec009 into mdn:main Jul 12, 2023
3 checks passed
@maximtop
Copy link
Contributor Author

Thank, @Rob--W

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:WebExt WebExtensions docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update declarativeNetRequest.RuleCondition
5 participants