Opened on 06/06/2018 at 03:41:20 PM
Closed on 06/19/2018 at 09:14:03 AM
Last modified on 09/30/2018 at 09:41:33 AM
#6735 closed change (fixed)
Store domains in lower case
| Reported by: | mjethani | Assignee: | mjethani |
|---|---|---|---|
| Priority: | P2 | Milestone: | |
| Module: | Core | Keywords: | |
| Cc: | sergz, kzar | Blocked By: | |
| Blocking: | #7000 | Platform: | Unknown / Cross platform |
| Ready: | yes | Confidential: | no |
| Tester: | Ross | Verified working: | yes |
| Review URL(s): | |||
Description (last modified by mjethani)
Background
Domain names are typically represented in lower case, both in filter lists and as reported by the browser. But the filter classes store the domain names in upper case. The conversion from lower case to upper case creates an additional string that would not otherwise be necessary.
According to my tests, if the filter classes store domain names in lower case instead, it saves ~2 MB on the JS heap with the default subscriptions.
What to change
In the RegExpFilter.fromText implementation, convert the value of the domain option to lower case rather than upper case. Update the names of related properties and the documentation accordingly.
Hints for testers
Test that filters with upper case or mixed case domains work correctly. For example, ExAmPlE.com,~WWW.EXAMPLE.COM##.foo should apply on https://example.com/ but not on https://www.example.com/
Attachments (0)
Change History (8)
comment:2 Changed on 06/07/2018 at 10:27:47 AM by mjethani
- Priority changed from Unknown to P2
- Ready set
- Status changed from new to reviewing
comment:3 Changed on 06/19/2018 at 09:13:05 AM by abpbot
comment:4 Changed on 06/19/2018 at 09:14:03 AM by mjethani
- Resolution set to fixed
- Status changed from reviewing to closed
comment:7 Changed on 08/21/2018 at 11:34:20 AM by Ross
- Tester changed from Unknown to Ross
- Verified working set
Working as described.
ABP 3.2.0.2103
Chrome 68 / 55 / 49 / Windows 10
Firefox 61 / 55 / 51 / Windows 10
comment:8 Changed on 09/30/2018 at 09:41:33 AM by mjethani
- Blocking 7000 added
A commit referencing this issue has landed:
Issue 6735 - Store domains in lower case