Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1294)

Issue 30046562: Issue 7458 - Duplicate entries for EasyPrivacy and Fanboy's Social Blocking List

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 months ago by diegocarloslima
Modified:
3 months ago
Reviewers:
anton, jens
Visibility:
Public.

Description

Issue 7458 - Duplicate entries for EasyPrivacy and Fanboy's Social Blocking List

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+36 lines, -40 lines) Patch
M mobile/android/base/java/org/adblockplus/browser/MoreSubscriptionsPreferenceGroup.java View 3 chunks +32 lines, -39 lines 2 comments Download
M mobile/android/base/java/org/adblockplus/browser/SubscriptionPreferenceCategory.java View 1 chunk +4 lines, -1 line 1 comment Download

Messages

Total messages: 3
diegocarloslima
3 months ago (2019-04-16 14:11:17 UTC) #1
jens
https://codereview.adblockplus.org/30046562/diff/30046563/mobile/android/base/java/org/adblockplus/browser/MoreSubscriptionsPreferenceGroup.java File mobile/android/base/java/org/adblockplus/browser/MoreSubscriptionsPreferenceGroup.java (right): https://codereview.adblockplus.org/30046562/diff/30046563/mobile/android/base/java/org/adblockplus/browser/MoreSubscriptionsPreferenceGroup.java#newcode83 mobile/android/base/java/org/adblockplus/browser/MoreSubscriptionsPreferenceGroup.java:83: if(!BUILTIN_URLS.contains(s.url)) The white space after the if is missing ...
3 months ago (2019-04-17 09:39:22 UTC) #2
anton
3 months ago (2019-04-17 09:46:35 UTC) #3
https://codereview.adblockplus.org/30046562/diff/30046563/mobile/android/base...
File
mobile/android/base/java/org/adblockplus/browser/MoreSubscriptionsPreferenceGroup.java
(right):

https://codereview.adblockplus.org/30046562/diff/30046563/mobile/android/base...
mobile/android/base/java/org/adblockplus/browser/MoreSubscriptionsPreferenceGroup.java:50:
"https://easylist-downloads.adblockplus.org/easyprivacy.txt",
the order of `BUILTIN_URLS` should be consistent to `BUILTIN_TITLES`.
It may be unclear without looking into how they are used.

So i'd suggest to either add note here (that order should be consistent) or
use `List<Pair>` where pair has `id` and `url` fields.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5