Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: lib/matcher.js

Issue 29965585: Issue 6992 - Try all keyword candidates when removing a filter (Closed) Base URL: https://hg.adblockplus.org/adblockpluscore/
Patch Set: Created Dec. 19, 2018, 5:56 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | test/matcher.js » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: lib/matcher.js
===================================================================
--- a/lib/matcher.js
+++ b/lib/matcher.js
@@ -105,35 +105,45 @@
}
}
/**
* Removes a filter by a given keyword from a map.
* @param {RegExpFilter} filter
* @param {string} keyword
* @param {Map.<string,(RegExpFilter|Set.<RegExpFilter>)>} map
+ * @returns {boolean} Whether the filter was removed.
*/
function removeFilterByKeyword(filter, keyword, map)
{
let set = map.get(keyword);
if (typeof set == "undefined")
- return;
+ return false;
if (set.size == 1)
{
- if (filter == set)
- map.delete(keyword);
+ if (filter != set)
+ return false;
+
+ map.delete(keyword);
}
else
{
+ let count = set.size;
+
set.delete(filter);
+ if (set.size == count)
+ return false;
+
if (set.size == 1)
map.set(keyword, [...set][0]);
}
+
+ return true;
}
/**
* Checks whether a particular filter is slow.
* @param {RegExpFilter} filter
* @returns {boolean}
*/
function isSlowFilter(filter)
@@ -211,24 +221,50 @@
}
/**
* Removes a filter from the matcher
* @param {RegExpFilter} filter
*/
remove(filter)
{
- let keyword = this.findKeyword(filter);
+ let keyword = null;
+ let candidates = null;
+
+ let {pattern} = filter;
+ if (pattern == null)
+ {
+ candidates = [""];
+ }
+ else
+ {
+ candidates = pattern.toLowerCase().match(allKeywordsRegExp);
+ if (!candidates)
+ candidates = [""];
+ }
+
let locationOnly = filter.isLocationOnly();
- removeFilterByKeyword(filter, keyword,
- locationOnly ? this._simpleFiltersByKeyword :
- this._complexFiltersByKeyword);
+ // The keyword used to add a filter depends on the current list of filters
+ // at the time of addition. At this point we don't know what keyword was
+ // used to add the filter. We must try all the candidates.
+ for (let candidate of candidates)
+ {
+ candidate = candidate.substring(1);
- if (locationOnly)
+ if (removeFilterByKeyword(filter, candidate,
+ locationOnly ? this._simpleFiltersByKeyword :
+ this._complexFiltersByKeyword))
+ {
+ keyword = candidate;
+ break;
+ }
+ }
+
+ if (locationOnly || keyword == null)
return;
for (let type of nonDefaultTypes(filter.contentType))
{
let map = this._filterMapsByType.get(type);
if (map)
removeFilterByKeyword(filter, keyword, map);
}
« no previous file with comments | « no previous file | test/matcher.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld