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

Issue 29805597: Issue 6699 - Support the "circumvention" filter list as default subscription (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 months ago by hub
Modified:
1 month, 3 weeks ago
Base URL:
https://hg.adblockplus.org/adblockpluschrome/
Visibility:
Public.

Description

Issue 6699 - Support the "circumvention" filter list as default subscription

Patch Set 1 #

Total comments: 34

Patch Set 2 : Updated comments/doc #

Total comments: 5

Patch Set 3 : Use "circumvention" instead of "cv" #

Total comments: 6

Patch Set 4 : More review fixes. #

Total comments: 2

Patch Set 5 : Use "==" instead of "in" #

Unified diffs Side-by-side diffs Delta from patch set Stats (+294 lines, -33 lines) Patch
M lib/subscriptionInit.js View 1 2 3 4 3 chunks +58 lines, -33 lines 0 comments Download
A qunit/subscriptions.xml View 1 2 3 1 chunk +173 lines, -0 lines 0 comments Download
A qunit/tests/subscriptionInit.js View 1 2 3 4 1 chunk +63 lines, -0 lines 0 comments Download

Messages

Total messages: 16
hub
2 months ago (2018-06-13 03:33:53 UTC) #1
hub
This implement the logic changes to select a default "cv" subscription. The UI changes are ...
2 months ago (2018-06-13 03:36:24 UTC) #2
Manish Jethani
https://codereview.adblockplus.org/29805597/diff/29805598/lib/subscriptionInit.js File lib/subscriptionInit.js (right): https://codereview.adblockplus.org/29805597/diff/29805598/lib/subscriptionInit.js#newcode90 lib/subscriptionInit.js:90: * Finds the element for the default ad blocking ...
2 months ago (2018-06-13 08:15:17 UTC) #3
Manish Jethani
https://codereview.adblockplus.org/29805597/diff/29805598/lib/subscriptionInit.js File lib/subscriptionInit.js (right): https://codereview.adblockplus.org/29805597/diff/29805598/lib/subscriptionInit.js#newcode128 lib/subscriptionInit.js:128: // If multiple items have a matching prefix of ...
2 months ago (2018-06-13 09:42:29 UTC) #4
Manish Jethani
https://codereview.adblockplus.org/29805597/diff/29805598/lib/subscriptionInit.js File lib/subscriptionInit.js (right): https://codereview.adblockplus.org/29805597/diff/29805598/lib/subscriptionInit.js#newcode115 lib/subscriptionInit.js:115: // The "ads" subscription is the one driving the ...
2 months ago (2018-06-13 13:34:02 UTC) #5
Manish Jethani
https://codereview.adblockplus.org/29805597/diff/29805598/qunit/tests/subscriptionInit.js File qunit/tests/subscriptionInit.js (right): https://codereview.adblockplus.org/29805597/diff/29805598/qunit/tests/subscriptionInit.js#newcode41 qunit/tests/subscriptionInit.js:41: .then(response => response.text()) This appears not to be mentioned ...
2 months ago (2018-06-13 13:45:39 UTC) #6
hub
updated patch https://codereview.adblockplus.org/29805597/diff/29805598/lib/subscriptionInit.js File lib/subscriptionInit.js (right): https://codereview.adblockplus.org/29805597/diff/29805598/lib/subscriptionInit.js#newcode90 lib/subscriptionInit.js:90: * Finds the element for the default ...
2 months ago (2018-06-13 16:33:37 UTC) #7
Manish Jethani
https://codereview.adblockplus.org/29805597/diff/29805598/lib/subscriptionInit.js File lib/subscriptionInit.js (right): https://codereview.adblockplus.org/29805597/diff/29805598/lib/subscriptionInit.js#newcode110 lib/subscriptionInit.js:110: if (subscriptionType in ["ads", "cv"] && !selectedItem[subscriptionType]) On 2018/06/13 ...
2 months ago (2018-06-13 16:53:10 UTC) #8
Manish Jethani
https://codereview.adblockplus.org/29805597/diff/29806566/lib/subscriptionInit.js File lib/subscriptionInit.js (right): https://codereview.adblockplus.org/29805597/diff/29806566/lib/subscriptionInit.js#newcode225 lib/subscriptionInit.js:225: for (let name in subs) By the way, if ...
2 months ago (2018-06-13 16:58:12 UTC) #9
hub
Updated patch https://codereview.adblockplus.org/29805597/diff/29805598/lib/subscriptionInit.js File lib/subscriptionInit.js (right): https://codereview.adblockplus.org/29805597/diff/29805598/lib/subscriptionInit.js#newcode139 lib/subscriptionInit.js:139: else if (subscriptionType == "cv") On 2018/06/13 ...
2 months ago (2018-06-13 21:58:19 UTC) #10
Manish Jethani
https://codereview.adblockplus.org/29805597/diff/29805598/lib/subscriptionInit.js File lib/subscriptionInit.js (right): https://codereview.adblockplus.org/29805597/diff/29805598/lib/subscriptionInit.js#newcode217 lib/subscriptionInit.js:217: let subs = chooseFilterSubscriptions(nodes); On 2018/06/13 21:58:18, hub wrote: ...
2 months ago (2018-06-14 05:07:34 UTC) #11
Manish Jethani
https://codereview.adblockplus.org/29805597/diff/29806566/lib/subscriptionInit.js File lib/subscriptionInit.js (right): https://codereview.adblockplus.org/29805597/diff/29806566/lib/subscriptionInit.js#newcode225 lib/subscriptionInit.js:225: for (let name in subs) On 2018/06/14 05:07:34, Manish ...
2 months ago (2018-06-14 05:42:57 UTC) #12
hub
Updated patch https://codereview.adblockplus.org/29805597/diff/29806566/lib/subscriptionInit.js File lib/subscriptionInit.js (right): https://codereview.adblockplus.org/29805597/diff/29806566/lib/subscriptionInit.js#newcode225 lib/subscriptionInit.js:225: for (let name in subs) On 2018/06/14 ...
2 months ago (2018-06-14 19:59:57 UTC) #13
Manish Jethani
Sorry about the delay. LGTM, except for the one comment below. https://codereview.adblockplus.org/29805597/diff/29807582/lib/subscriptionInit.js File lib/subscriptionInit.js (right): ...
1 month, 3 weeks ago (2018-06-19 09:23:31 UTC) #14
hub
Updated patch. https://codereview.adblockplus.org/29805597/diff/29807582/lib/subscriptionInit.js File lib/subscriptionInit.js (right): https://codereview.adblockplus.org/29805597/diff/29807582/lib/subscriptionInit.js#newcode115 lib/subscriptionInit.js:115: if (subscriptionType in ["ads", "circumvention"] && On ...
1 month, 3 weeks ago (2018-06-19 12:41:14 UTC) #15
Manish Jethani
1 month, 3 weeks ago (2018-06-19 12:44:56 UTC) #16
LGTM
Sign in to reply to this message.

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