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

Unified Diff: lib/subscriptionInit.js

Issue 29805597: Issue 6699 - Support the "circumvention" filter list as default subscription (Closed) Base URL: https://hg.adblockplus.org/adblockpluschrome/
Patch Set: Updated comments/doc Created June 13, 2018, 4:32 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | qunit/subscriptions.xml » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: lib/subscriptionInit.js
===================================================================
--- a/lib/subscriptionInit.js
+++ b/lib/subscriptionInit.js
@@ -82,61 +82,74 @@
subscription.filters.length > 0)
return false;
}
return true;
}
/**
- * Finds the element for the default ad blocking filter subscription based
+ * @typedef {object} DefaultSubscriptions
+ * @property {?Element} ads
+ * @property {?Element} cv
+ */
+/**
+ * Finds the elements for the default ad blocking filter subscriptions based
* on the user's locale.
*
* @param {HTMLCollection} subscriptions
- * @return {Element}
+ * @return {DefaultSubscriptions}
*/
-function chooseFilterSubscription(subscriptions)
+function chooseFilterSubscriptions(subscriptions)
{
- let selectedItem = null;
+ let selectedItem = {};
let selectedPrefix = null;
let matchCount = 0;
for (let subscription of subscriptions)
{
- if (!selectedItem)
- selectedItem = subscription;
-
let prefixes = subscription.getAttribute("prefixes");
let prefix = prefixes && prefixes.split(",").find(
lang => new RegExp("^" + lang + "\\b").test(Utils.appLocale)
);
let subscriptionType = subscription.getAttribute("type");
- if (prefix && subscriptionType == "ads")
+ if (subscriptionType in ["ads", "cv"] && !selectedItem[subscriptionType])
+ selectedItem[subscriptionType] = subscription;
+
+ if (prefix)
{
- if (!selectedPrefix || selectedPrefix.length < prefix.length)
- {
- selectedItem = subscription;
- selectedPrefix = prefix;
- matchCount = 1;
- }
- else if (selectedPrefix && selectedPrefix.length == prefix.length)
+ // The "ads" subscription is the one driving the selection.
+ if (subscriptionType == "ads")
{
- matchCount++;
+ if (!selectedPrefix || selectedPrefix.length < prefix.length)
+ {
+ selectedItem[subscriptionType] = subscription;
+ selectedPrefix = prefix;
+ matchCount = 1;
+ }
+ else if (selectedPrefix && selectedPrefix.length == prefix.length)
+ {
+ matchCount++;
- // If multiple items have a matching prefix of the same length:
- // Select one of the items randomly, probability should be the same
- // for all items. So we replace the previous match here with
- // probability 1/N (N being the number of matches).
- if (Math.random() * matchCount < 1)
- {
- selectedItem = subscription;
- selectedPrefix = prefix;
+ // If multiple items have a matching prefix of the same length:
+ // Select one of the items randomly, probability should be the same
+ // for all items. So we replace the previous match here with
+ // probability 1/N (N being the number of matches).
+ if (Math.random() * matchCount < 1)
+ {
+ selectedItem[subscriptionType] = subscription;
+ selectedPrefix = prefix;
+ }
}
}
+ else if (subscriptionType == "cv")
+ {
+ selectedItem[subscriptionType] = subscription;
+ }
}
}
return selectedItem;
}
function supportsNotificationsWithButtons()
{
// Microsoft Edge (as of EdgeHTML 16) doesn't have the notifications API.
@@ -201,27 +214,35 @@
{
return fetch("subscriptions.xml")
.then(response => response.text())
.then(text =>
{
let doc = new DOMParser().parseFromString(text, "application/xml");
let nodes = doc.getElementsByTagName("subscription");
- let node = chooseFilterSubscription(nodes);
- if (node)
+ let subs = chooseFilterSubscriptions(nodes);
+ if (subs)
{
- let url = node.getAttribute("url");
- if (url)
+ for (let name in subs)
Manish Jethani 2018/06/13 16:58:12 By the way, if you want to keep the return type a
hub 2018/06/13 21:58:19 There are at most two properties.
Manish Jethani 2018/06/14 05:07:34 Acknowledged. Since the calling code is now assum
Manish Jethani 2018/06/14 05:42:56 OK, forget this, it makes it worse. Like you said
hub 2018/06/14 19:59:57 Acknowledged.
{
- let subscription = Subscription.fromURL(url);
- subscription.disabled = false;
- subscription.title = node.getAttribute("title");
- subscription.homepage = node.getAttribute("homepage");
- subscriptions.push(subscription);
+ let node = subs[name];
+ if (!node)
+ continue;
+
+ let url = node.getAttribute("url");
+ if (url)
+ {
+ let subscription = Subscription.fromURL(url);
+ subscription.disabled = false;
+ subscription.title = node.getAttribute("title");
+ subscription.homepage = node.getAttribute("homepage");
+ subscription.type = node.getAttribute("type");
+ subscriptions.push(subscription);
+ }
}
}
return subscriptions;
});
}
return subscriptions;
@@ -301,8 +322,11 @@
* that will effectively be added.
*
* @param {function} callback
*/
exports.setSubscriptionsCallback = callback =>
{
subscriptionsCallback = callback;
};
+
+// Exports for tests only
+exports.chooseFilterSubscriptions = chooseFilterSubscriptions;
« no previous file with comments | « no previous file | qunit/subscriptions.xml » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld