Changes between Version 12 and Version 14 of Ticket #6994


Ignore:
Timestamp:
10/22/2018 02:17:34 AM (12 months ago)
Author:
jsonesen
Comment:

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #6994

    • Property Status changed from new to reviewing
  • Ticket #6994 – Description

    v12 v14  
    55 
    66=== What to change === 
    7 Add a `isLocationOnly` property to the RegExpFilter class and a new `locationFilterByKeyword` to the Matcher constructor. Also filters must not be stored twice so when adding fastfilter-by-keyword skip adding to the filter-by-keyword map. 
     7Add a new map for location only filters (`fastFilterByKeyword`) to the matcher class in `lib/matcher.js`  
    88 
    9 Change `_checkEntryMatch` in `lib/matcher.js` to initially loop through location only filters checking for matches using `location.test(filter.regexp)` Additionally, in `_checkEntryMatch`, if `typeMask & RegExpFilter.prototype.contentType == 0` is true, these filters should be skipped entirely. 
     9 
     10Change `_checkEntryMatch` in lib/matcher.js to initially loop through location only filters checking for matches using filter.regexp.test(location) or if a new location only version of `RegExpFilter.prototype.matches` is used it can call filter.matchesLocation. Additionally, in _checkEntryMatch, if  
     11`typeMask == 0 && RegExpFilter.prototype.contentType == 0`, these filters should be skipped entirely. 
     12 
     13Add a isLocationOnly() property to the RegExpFilter class. Also, filters must not be stored twice so when adding to the fastFilterByKeyword map we should skip adding to the `matcher.filterByKeyword` map. 
     14 
     15Using a new map in the matcher class will require the following changes: 
     16 
     17 - `Matcher.findKeyword` will have to be updated to handle fast-filters (location only) 
     18 - `Matcher.add()` and `Matcher.remove(filter)` will have to determine which map to use based on the new `isLocationOnly` method from `RegExpFilter` 
     19 
     20=== Notes === 
     21While the goal is to not change anything about the tests, changing the findkeyword method as well as having two maps will likely require changes to keyword tests and anywhere filterByKeyword is used. 
  • Ticket #6994 – Review URL(s)

    v12 v14  
     1https://codereview.adblockplus.org/29907586/