Left: | ||
Right: |
LEFT | RIGHT |
---|---|
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-present eyeo GmbH | 3 * Copyright (C) 2006-present 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 /* globals publicSuffixes */ | 18 /* globals publicSuffixes */ |
Thomas Greiner
2018/05/30 10:18:03
Detail: This whitespace appears to have been added
kzar
2018/05/30 10:57:48
The newline? If so it seems consistent with how we
Thomas Greiner
2018/05/30 12:30:09
No, the whitespace character to the left of the co
kzar
2018/05/30 12:59:02
Oh yea, I see what you mean now.
Jon Sonesen
2018/05/30 15:46:54
Acknowledged.
| |
19 | 19 |
20 "use strict"; | 20 "use strict"; |
21 | 21 |
22 const {Prefs} = require("prefs"); | 22 const {Prefs} = require("prefs"); |
23 const {ActiveFilter} = require("filterClasses"); | 23 const {ActiveFilter} = require("filterClasses"); |
24 const {FilterStorage} = require("filterStorage"); | 24 const {FilterStorage} = require("filterStorage"); |
25 const {FilterNotifier} = require("filterNotifier"); | 25 const {FilterNotifier} = require("filterNotifier"); |
26 const {Subscription} = require("subscriptionClasses"); | 26 const {Subscription} = require("subscriptionClasses"); |
27 const {Notification} = require("notification"); | 27 const {Notification} = require("notification"); |
28 | 28 |
(...skipping 19 matching lines...) Expand all Loading... | |
48 let urlFilters = []; | 48 let urlFilters = []; |
49 for (let filter of subscription.filters) | 49 for (let filter of subscription.filters) |
50 { | 50 { |
51 if (filter instanceof ActiveFilter && filter.domains) | 51 if (filter instanceof ActiveFilter && filter.domains) |
52 { | 52 { |
53 for (let [domain, included] of filter.domains) | 53 for (let [domain, included] of filter.domains) |
54 { | 54 { |
55 let urlFilter = "||" + domain + "^$document"; | 55 let urlFilter = "||" + domain + "^$document"; |
56 if (domain && included && urlFilters.indexOf(urlFilter) == -1) | 56 if (domain && included && urlFilters.indexOf(urlFilter) == -1) |
57 { | 57 { |
58 if (domain.indexOf(".") != -1 || domain.endsWith(".")) | 58 if (/\.[^$]/.test(domain) && !publicSuffixes.hasOwnProperty(domain)) |
Thomas Greiner
2018/05/23 13:45:54
Effectively, this logic leads to the following res
Thomas Greiner
2018/05/23 13:45:54
Suggestion: Double negation usually makes code qui
kzar
2018/05/23 14:41:45
Also please can you use `.includes(".")` instead o
Jon Sonesen
2018/05/23 23:12:51
For sure
Jon Sonesen
2018/05/23 23:12:51
Acknowledged.
Jon Sonesen
2018/05/23 23:12:52
Hey,
My bad I think I misunderstood the last comm
Jon Sonesen
2018/05/24 00:00:09
Actually, on second thought would it be better to
kzar
2018/05/24 11:28:57
I'm not sure I understand the question. Why do you
|
kzar
2018/05/30 10:57:48
Please combine the two if statements. Also please
Thomas Greiner
2018/05/30 12:30:09
Good call. This regex will indeed fail for multi-p
kzar
2018/05/30 12:59:02
Well my concern was that "co.uk" is in the publicS
Jon Sonesen
2018/05/30 15:46:54
Unit tests it is then!
|
59 { | 59 { |
60 if (!(publicSuffixes.hasOwnProperty(domain))) | 60 urlFilters.push(urlFilter); |
61 urlFilters.push(urlFilter); | |
62 } | 61 } |
63 } | 62 } |
64 } | 63 } |
65 } | 64 } |
66 } | 65 } |
67 notification.urlFilters = urlFilters; | 66 notification.urlFilters = urlFilters; |
68 Notification.addNotification(notification); | 67 Notification.addNotification(notification); |
69 Notification.addQuestionListener(notification.id, notificationListener); | 68 Notification.addQuestionListener(notification.id, notificationListener); |
70 } | 69 } |
71 | 70 |
(...skipping 18 matching lines...) Expand all Loading... | |
90 if (url in FilterStorage.knownSubscriptions && subscription.disabled) | 89 if (url in FilterStorage.knownSubscriptions && subscription.disabled) |
91 addAntiAdblockNotification(subscription); | 90 addAntiAdblockNotification(subscription); |
92 else | 91 else |
93 removeAntiAdblockNotification(); | 92 removeAntiAdblockNotification(); |
94 } | 93 } |
95 | 94 |
96 FilterNotifier.on("subscription.updated", onSubscriptionChange); | 95 FilterNotifier.on("subscription.updated", onSubscriptionChange); |
97 FilterNotifier.on("subscription.removed", onSubscriptionChange); | 96 FilterNotifier.on("subscription.removed", onSubscriptionChange); |
98 FilterNotifier.on("subscription.disabled", onSubscriptionChange); | 97 FilterNotifier.on("subscription.disabled", onSubscriptionChange); |
99 }; | 98 }; |
LEFT | RIGHT |