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

Delta Between Two Patch Sets: lib/filterClasses.js

Issue 29361668: Issue 4394 - Create a filter class for element hiding emulation filters (Closed) Base URL: https://bitbucket.org/fhd/adblockpluscore
Left Patch Set: Created Nov. 3, 2016, 3:42 p.m.
Right Patch Set: Improve compliance with the 80 column rule Created Nov. 21, 2016, 8:10 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
« no previous file with change/comment | « lib/elemHideEmulation.js ('k') | lib/filterListener.js » ('j') | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 /* 1 /*
2 * This file is part of Adblock Plus <https://adblockplus.org/>, 2 * This file is part of Adblock Plus <https://adblockplus.org/>,
3 * Copyright (C) 2006-2016 Eyeo GmbH 3 * Copyright (C) 2006-2016 Eyeo GmbH
4 * 4 *
5 * Adblock Plus is free software: you can redistribute it and/or modify 5 * Adblock Plus is free software: you can redistribute it and/or modify
6 * it under the terms of the GNU General Public License version 3 as 6 * it under the terms of the GNU General Public License version 3 as
7 * published by the Free Software Foundation. 7 * published by the Free Software Foundation.
8 * 8 *
9 * Adblock Plus is distributed in the hope that it will be useful, 9 * Adblock Plus is distributed in the hope that it will be useful,
10 * but WITHOUT ANY WARRANTY; without even the implied warranty of 10 * but WITHOUT ANY WARRANTY; without even the implied warranty of
11 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the 11 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12 * GNU General Public License for more details. 12 * GNU General Public License for more details.
13 * 13 *
14 * You should have received a copy of the GNU General Public License 14 * You should have received a copy of the GNU General Public License
15 * along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>. 15 * along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>.
16 */ 16 */
17 17
18 /** 18 /**
19 * @fileOverview Definition of Filter class and its subclasses. 19 * @fileOverview Definition of Filter class and its subclasses.
20 */ 20 */
21 21
22 let {FilterNotifier} = require("filterNotifier"); 22 let {FilterNotifier} = require("filterNotifier");
23 let {extend} = require("coreUtils"); 23 let {extend} = require("coreUtils");
24 let {elemHideEmulationFeatureMap, filterToRegExp} = require("shared"); 24 let {filterToRegExp} = require("common");
25 25
26 /** 26 /**
27 * Abstract base class for filters 27 * Abstract base class for filters
28 * 28 *
29 * @param {String} text string representation of the filter 29 * @param {String} text string representation of the filter
30 * @constructor 30 * @constructor
31 */ 31 */
32 function Filter(text) 32 function Filter(text)
33 { 33 {
34 this.text = text; 34 this.text = text;
(...skipping 870 matching lines...) Expand 10 before | Expand all | Expand 10 after
905 905
906 // We don't allow ElemHide filters which have any empty domains. 906 // We don't allow ElemHide filters which have any empty domains.
907 // Note: The ElemHide.prototype.domainSeparator is duplicated here, if that 907 // Note: The ElemHide.prototype.domainSeparator is duplicated here, if that
908 // changes this must be changed too. 908 // changes this must be changed too.
909 if (domain && /(^|,)~?(,|$)/.test(domain)) 909 if (domain && /(^|,)~?(,|$)/.test(domain))
910 return new InvalidFilter(text, "filter_invalid_domain"); 910 return new InvalidFilter(text, "filter_invalid_domain");
911 911
912 if (isException) 912 if (isException)
913 return new ElemHideException(text, domain, selector); 913 return new ElemHideException(text, domain, selector);
914 914
915 let emulatedFeatures = 0;
916 if (selector.indexOf("[-abp-properties") != -1) 915 if (selector.indexOf("[-abp-properties") != -1)
917 emulatedFeatures |= ElemHideEmulationFilter.featureMap.PROPERTY_SELECTOR;
918 if (selector.indexOf(":has(") != -1)
kzar 2016/11/04 15:45:56 The feature flags are a nice idea but I wonder wha
Felix Dahlke 2016/11/04 16:43:35 Well, if we detect NO feature here, this would jus
kzar 2016/11/07 12:36:26 I guess my question is what's the point of having
Felix Dahlke 2016/11/07 14:41:24 Well, we have to check for emulated features befor
kzar 2016/11/07 15:59:44 Well we don't need to set a boolean flag here eith
Felix Dahlke 2016/11/07 16:51:24 Yeah of course, my bad.
kzar 2016/11/07 17:10:22 That's not quite true, since if we're recording us
Felix Dahlke 2016/11/08 17:45:35 Well, sure, we don't have to run all checks if one
kzar 2016/11/15 16:54:13 Well no I think that if the only reason to add all
Felix Dahlke 2016/11/15 21:11:44 I still think it makes sense to start detecting (y
Wladimir Palant 2016/11/21 11:39:14 I actually have to agree with Dave here. You are d
Felix Dahlke 2016/11/21 14:38:58 Fair enough, I guess if we're not worried about ba
919 emulatedFeatures |= ElemHideEmulationFilter.featureMap.HAS_PSEUDO_CLASS;
920
921 if (emulatedFeatures != 0)
922 { 916 {
923 // Element hiding emulation filters are inefficient so we need to make sure 917 // Element hiding emulation filters are inefficient so we need to make sure
924 // that they're only applied if they specify active domains 918 // that they're only applied if they specify active domains
925 if (!/,[^~][^,.]*\.[^,]/.test("," + domain)) 919 if (!/,[^~][^,.]*\.[^,]/.test("," + domain))
926 return new InvalidFilter(text, "filter_elemhideemulation_nodomain"); 920 return new InvalidFilter(text, "filter_elemhideemulation_nodomain");
927 921
928 return new ElemHideEmulationFilter(text, domain, selector, 922 return new ElemHideEmulationFilter(text, domain, selector);
kzar 2016/11/04 15:45:56 I wonder if it's really a good idea to move the lo
Felix Dahlke 2016/11/04 16:43:35 Well, one part of the truth is that I don't know h
kzar 2016/11/07 17:19:25 That's a pretty good point actually, I didn't thin
929 emulatedFeatures);
930 } 923 }
931 924
932 return new ElemHideFilter(text, domain, selector); 925 return new ElemHideFilter(text, domain, selector);
933 }; 926 };
934 927
935 /** 928 /**
936 * Class for element hiding filters 929 * Class for element hiding filters
937 * @param {String} text see Filter() 930 * @param {String} text see Filter()
938 * @param {String} domains see ElemHideBase() 931 * @param {String} domains see ElemHideBase()
939 * @param {String} selector see ElemHideBase() 932 * @param {String} selector see ElemHideBase()
(...skipping 26 matching lines...) Expand all
966 959
967 ElemHideException.prototype = extend(ElemHideBase, { 960 ElemHideException.prototype = extend(ElemHideBase, {
968 type: "elemhideexception" 961 type: "elemhideexception"
969 }); 962 });
970 963
971 /** 964 /**
972 * Class for element hiding emulation filters 965 * Class for element hiding emulation filters
973 * @param {String} text see Filter() 966 * @param {String} text see Filter()
974 * @param {String} domains see ElemHideBase() 967 * @param {String} domains see ElemHideBase()
975 * @param {String} selector see ElemHideBase() 968 * @param {String} selector see ElemHideBase()
976 * @param {Integer} features see ElemHideEmulationFilter.features
977 * @constructor 969 * @constructor
978 * @augments ElemHideBase 970 * @augments ElemHideBase
979 */ 971 */
980 function ElemHideEmulationFilter(text, domains, selector, features) 972 function ElemHideEmulationFilter(text, domains, selector)
981 { 973 {
982 ElemHideBase.call(this, text, domains, selector); 974 ElemHideBase.call(this, text, domains, selector);
983
984 this.features = features;
985 } 975 }
986 exports.ElemHideEmulationFilter = ElemHideEmulationFilter; 976 exports.ElemHideEmulationFilter = ElemHideEmulationFilter;
987 977
988 ElemHideEmulationFilter.prototype = extend(ElemHideBase, { 978 ElemHideEmulationFilter.prototype = extend(ElemHideBase, {
989 type: "elemhideemulation", 979 type: "elemhideemulation"
990 980 });
991 /**
992 * Features used in this filter, combination of values from
kzar 2016/11/04 15:45:56 Perhaps describe it as bit flags instead of combin
Felix Dahlke 2016/11/04 16:43:35 I thought I'd keep this in line with the doc comme
kzar 2016/11/07 17:19:25 Well bitmap works too. (I guess if you're worried
993 * ElemHideEmulationFilter.featureMap
994 * @type Integer
995 */
996 features: 0
997 });
998
999 /**
1000 * @see elemHideEmulationFeatureMap
1001 */
1002 ElemHideEmulationFilter.featureMap = elemHideEmulationFeatureMap;
LEFTRIGHT

Powered by Google App Engine
This is Rietveld