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

Delta Between Two Patch Sets: lib/filterClasses.js

Issue 29361640: Issue 4593 - Support regular expressions for CSS property matching (Closed) Base URL: https://bitbucket.org/fhd/adblockpluscore
Left Patch Set: Created Nov. 3, 2016, 11:40 a.m.
Right Patch Set: Don't detect '/' as a regexp and speed up regexp detection Created Nov. 8, 2016, 5:28 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 | « no previous file | test/filterClasses.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
(...skipping 1025 matching lines...) Expand 10 before | Expand all | Expand 10 after
1036 */ 1036 */
1037 get regexpString() 1037 get regexpString()
1038 { 1038 {
1039 // Despite this property being cached, the getter is called 1039 // Despite this property being cached, the getter is called
1040 // several times on Safari, due to WebKit bug 132872 1040 // several times on Safari, due to WebKit bug 132872
1041 let prop = Object.getOwnPropertyDescriptor(this, "regexpString"); 1041 let prop = Object.getOwnPropertyDescriptor(this, "regexpString");
1042 if (prop) 1042 if (prop)
1043 return prop.value; 1043 return prop.value;
1044 1044
1045 let regexp; 1045 let regexp;
1046 if (this.regexpSource.charAt(0) == "/" && 1046 if (this.regexpSource.length >= 2 && this.regexpSource[0] == "/" &&
kzar 2016/11/03 12:18:16 Nit: How about using startsWith and endsWith inste
Felix Dahlke 2016/11/03 13:47:51 Wouldn't work on Safari <9, would it? Maybe this i
kzar 2016/11/03 16:03:19 Right, we wouldn't be able to update the adblockpl
Felix Dahlke 2016/11/03 19:08:02 Alright, seems safe to break Safari <9 then - done
1047 this.regexpSource.slice(-1) == "/") 1047 this.regexpSource[this.regexpSource.length - 1] == "/")
Wladimir Palant 2016/11/08 17:41:34 Well, accessing this.regexpSource four times is ob
1048 regexp = this.regexpSource.slice(1, -1); 1048 regexp = this.regexpSource.slice(1, -1);
kzar 2016/11/03 12:18:16 What about flags, for example for case-insensitive
Felix Dahlke 2016/11/03 13:47:51 Hm, good point. How I see it, `i` is actually the
kzar 2016/11/03 16:03:19 Yea, I think you're right there. I thought perhaps
Felix Dahlke 2016/11/03 19:08:02 I dug a bit deeper and while I was apparently righ
kzar 2016/11/04 10:06:56 I would prefer to switch to case insensitive match
Felix Dahlke 2016/11/04 13:43:56 I know it'd be a simple change and I agree we shou
kzar 2016/11/04 14:29:50 It's not really an unrelated change IMO, since lik
Felix Dahlke 2016/11/04 16:11:44 Well, it's mainly for consistency with how our oth
kzar 2016/11/04 16:18:27 Yea they do, I checked that.
Felix Dahlke 2016/11/04 16:51:16 URL matching is _always_ case insensitive, whether
1049 else 1049 else
1050 regexp = Filter.toRegExp(this.regexpSource); 1050 regexp = Filter.toRegExp(this.regexpSource);
1051 Object.defineProperty(this, "regexpString", {value: regexp}); 1051 Object.defineProperty(this, "regexpString", {value: regexp});
1052 return regexp; 1052 return regexp;
1053 } 1053 }
1054 }); 1054 });
LEFTRIGHT

Powered by Google App Engine
This is Rietveld