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

declarativeNetRequest API: have isUrlFilterCaseSensitive condition set to false by default #269

Closed
gorhill opened this issue Sep 7, 2022 · 9 comments
Labels
implemented: chrome Implemented in Chrome implemented: firefox Implemented in Firefox implemented: safari Implemented in Safari proposal Proposal for a change or new feature topic: dnr Related to declarativeNetRequest

Comments

@gorhill
Copy link

gorhill commented Sep 7, 2022

Currently, the isUrlFilterCaseSensitive condition defaults to true when not specified.

I propose the default state to be changed to false.

Content blockers are probably behind the largest rulesets which will feed into the declarativeNetRequest API, and top content blockers are compatible and abide by EasyList syntax. In EasyList syntax, the pattern matching is case-insensitive by default, and the match-case option is rarely used.

At the moment, I find zero instance of match-case in EasyList and EasyPrivacy. I find 23 instances in uBlock Origin's own filter lists, and 5 instances in AdGuard's English list.

Defaulting isUrlFilterCaseSensitive to false would avoid the need to encode the property for the vast majority of rules, and thus reduce the size and verbosity of ruleset files.

For example, when I convert the default filter lists of uBO into a json ruleset file, the result is ~3.7 MB, while it is ~3 MB when assuming isUrlFilterCaseSensitive default to false (this was a single unformatted line of json).

@xeenon xeenon added proposal Proposal for a change or new feature supportive: safari Supportive from Safari labels Sep 8, 2022
@xeenon
Copy link
Collaborator

xeenon commented Sep 8, 2022

I support this from Apple. Our internal format already is case-insensitive by default.

@dotproto dotproto added the follow-up: chrome Needs a response from a Chrome representative label Sep 15, 2022
@dotproto
Copy link
Member

Thanks for the feedback, @gorhill. My first impression is that this seems like a good change. I suspect that the main challenge here is going to be breaking published extensions, tough we may be able to largely mitigate this through public announcements and direct outreach.

@carlosjeurissen carlosjeurissen added the topic: dnr Related to declarativeNetRequest label Sep 17, 2022
@Rob--W Rob--W added the supportive: firefox Supportive from Firefox label Sep 22, 2022
@zombie
Copy link
Collaborator

zombie commented Jan 19, 2023

Firefox also support this proposal, the risk of "breaking" existing extensions here seems minimal, as the new default would be to block more. It seems likely that those who care about case sensitivity would explicitly use the true condition, and those that don't (want/care about case sensitivity) would be less likely to include the explicit value for this condition.

@Rob--W Rob--W added implemented: firefox Implemented in Firefox and removed supportive: firefox Supportive from Firefox labels Jan 20, 2023
@Rob--W
Copy link
Member

Rob--W commented Jan 20, 2023

Firefox has already implemented this default: https://bugzilla.mozilla.org/show_bug.cgi?id=1811498

Safari has already filed a tracking issue internally.

Chrome will also file such a bug.

Relevant part of yesterday's meeting notes:

[Issue 269](https://github.com/w3c/webextensions/issues/269): declarativeNetRequest API: have isUrlFilterCaseSensitive condition set to false by default
* [timothy] We were all in favor and were waiting for Simeon to follow up with Chrome.
* [simeon] We are onboard with the change. There was a concern with compiled vs not compiled, but we are still onboard. This is a breaking change, so we need to get a word out. That outreach is the primary blocker on the Chrome side.
* [tomislav] The proposed default has a lower risk of breaking than switching the defaults around.
* [andrey] There are few extensions.
* [rob] So we have repeated our approval of this proposal. Can we track these changes on our own issue trackers to make sure that this actually gets implemented? Especially with the few extensions potentially impacted right now, it makes sense to.
* [timothy] I filed a Safari-side issue.
* [simeon] Scheduling this on my side.

@xeenon xeenon added implemented: safari Implemented in Safari and removed supportive: safari Supportive from Safari labels Feb 8, 2023
@xeenon
Copy link
Collaborator

xeenon commented Feb 8, 2023

This shipped today in Safari Technology Preview 163.

@dotproto
Copy link
Member

dotproto commented Feb 9, 2023

I just opened https://crbug.com/1414441 to track this issue in Chromium.

@oliverdunk oliverdunk added supportive: chrome Supportive from Chrome and removed follow-up: chrome Needs a response from a Chrome representative labels Feb 24, 2023
@oliverdunk
Copy link
Member

Marking this as supportive: chrome Supportive from Chrome . Simeon mentioned in a previous meeting (https://github.com/w3c/webextensions/blob/main/_minutes/2023-01-19-wecg.md?plain=1#L119) but we are happy to make this change provided we've done some outreach first to give everyone a heads up. I'm going to look in to that.

@oliverdunk
Copy link
Member

We've done an announcement to developers here: https://groups.google.com/a/chromium.org/g/chromium-extensions/c/TXYUmvDHUlw/m/9JdxXG2cAgAJ. We're planning to make this change in M118.

Bug: https://crbug.com/1414441

Rob--W added a commit to mdn/content that referenced this issue Jul 12, 2023
* It is agreed to set isUrlFilterCaseSensitive to 'false' by default 

w3c/webextensions#269 (comment)

* updated info which browsers set isUrlFilterCaseSensitive to false by default

* Apply suggestion

Co-authored-by: Oliver Dunk <[email protected]>

* Update files/en-us/mozilla/add-ons/webextensions/api/declarativenetrequest/rulecondition/index.md

Co-authored-by: Rob Wu <[email protected]>

---------

Co-authored-by: Oliver Dunk <[email protected]>
Co-authored-by: Rob Wu <[email protected]>
@oliverdunk oliverdunk removed the supportive: chrome Supportive from Chrome label Nov 1, 2023
@oliverdunk oliverdunk added the implemented: chrome Implemented in Chrome label Nov 1, 2023
@oliverdunk
Copy link
Member

This shipped in Chrome 118: https://chromiumdash.appspot.com/commit/d90e6a56d0e77ce5d278a5b070098c5d8f7081fd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
implemented: chrome Implemented in Chrome implemented: firefox Implemented in Firefox implemented: safari Implemented in Safari proposal Proposal for a change or new feature topic: dnr Related to declarativeNetRequest
Projects
None yet
Development

No branches or pull requests

7 participants