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

Delta Between Two Patch Sets: lib/abp2blocklist.js

Issue 29443587: Noissue - Do not add subdomain wildcard if subdomain excluded (Closed) Base URL: https://hg.adblockplus.org/abp2blocklist
Left Patch Set: Add "www" prefix Created May 21, 2017, 5:39 p.m.
Right Patch Set: Rebase Created May 23, 2017, 5:03 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/abp2blocklist.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-2017 eyeo GmbH 3 * Copyright (C) 2006-2017 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 272 matching lines...) Expand 10 before | Expand all | Expand 10 after
283 283
284 if (included.length > 0) 284 if (included.length > 0)
285 { 285 {
286 trigger["if-domain"] = []; 286 trigger["if-domain"] = [];
287 287
288 for (let name of included) 288 for (let name of included)
289 { 289 {
290 // If this is a blocking filter or an element hiding filter, add the 290 // If this is a blocking filter or an element hiding filter, add the
291 // subdomain wildcard only if no subdomains have been excluded. 291 // subdomain wildcard only if no subdomains have been excluded.
292 let notSubdomains = null; 292 let notSubdomains = null;
293 if ((filter instanceof filterClasses.BlockingFilter || 293 if ((filter instanceof filterClasses.BlockingFilter ||
Sebastian Noack 2017/05/21 20:56:13 Ẁhy do we have to explicitly check for BlockingFil
Manish Jethani 2017/05/21 21:49:05 Because what is desired in the case of whitelistin
Sebastian Noack 2017/05/22 08:06:11 That might be a good point. It seems to be better
294 filter instanceof filterClasses.ElemHideFilter) && 294 filter instanceof filterClasses.ElemHideFilter) &&
295 (notSubdomains = findSubdomainsInList(name, excluded)).length > 0) 295 (notSubdomains = findSubdomainsInList(name, excluded)).length > 0)
296 { 296 {
297 trigger["if-domain"].push(name); 297 trigger["if-domain"].push(name);
298 298
299 // Add the "www" prefix but only if it hasn't been excluded. 299 // Add the "www" prefix but only if it hasn't been excluded.
300 if (!notSubdomains.some(name => name == "www")) 300 if (!notSubdomains.includes("www"))
Sebastian Noack 2017/05/21 20:56:13 Is this check actually correct? What if the domain
Manish Jethani 2017/05/21 21:49:05 Can you give a more complete example of where this
Sebastian Noack 2017/05/22 08:06:11 So in case of a filter like this: $domain=example
Manish Jethani 2017/05/22 10:37:39 In this case the code would be comparing with "foo
Sebastian Noack 2017/05/22 11:08:59 Alright. But I think it still can be slightly simp
Manish Jethani 2017/05/22 16:07:08 Done.
301 trigger["if-domain"].push("www." + name); 301 trigger["if-domain"].push("www." + name);
302 } 302 }
303 else 303 else
304 { 304 {
305 trigger["if-domain"].push("*" + name); 305 trigger["if-domain"].push("*" + name);
306 } 306 }
307 } 307 }
308 } 308 }
309 else if (excluded.length > 0) 309 else if (excluded.length > 0)
310 { 310 {
(...skipping 71 matching lines...) Expand 10 before | Expand all | Expand 10 after
382 { 382 {
383 newSelector.push(selector.substring(i, pos.start)); 383 newSelector.push(selector.substring(i, pos.start));
384 newSelector.push('[id=', selector.substring(pos.start + 1, pos.end), ']'); 384 newSelector.push('[id=', selector.substring(pos.start + 1, pos.end), ']');
385 i = pos.end; 385 i = pos.end;
386 } 386 }
387 newSelector.push(selector.substring(i)); 387 newSelector.push(selector.substring(i));
388 388
389 return newSelector.join(""); 389 return newSelector.join("");
390 } 390 }
391 391
392 function addCSSRules(rules, selectors, matchDomain)
393 {
394 while (selectors.length)
395 {
396 let selector = selectors.splice(0, selectorLimit).join(", ");
397
398 // As of Safari 9.0 element IDs are matched as lowercase. We work around
399 // this by converting to the attribute format [id="elementID"]
400 selector = convertIDSelectorsToAttributeSelectors(selector);
401
402 rules.push({
403 trigger: {"url-filter": matchDomain,
404 "url-filter-is-case-sensitive": true},
405 action: {type: "css-display-none",
406 selector: selector}
407 });
408 }
409 }
410
392 let ContentBlockerList = 411 let ContentBlockerList =
393 /** 412 /**
394 * Create a new Adblock Plus filter to content blocker list converter 413 * Create a new Adblock Plus filter to content blocker list converter
395 * 414 *
396 * @constructor 415 * @constructor
397 */ 416 */
398 exports.ContentBlockerList = function () 417 exports.ContentBlockerList = function ()
399 { 418 {
400 this.requestFilters = []; 419 this.requestFilters = [];
401 this.requestExceptions = []; 420 this.requestExceptions = [];
402 this.elemhideFilters = []; 421 this.elemhideFilters = [];
403 this.elemhideExceptions = []; 422 this.elemhideExceptions = [];
423 this.generichideExceptions = [];
404 this.elemhideSelectorExceptions = new Map(); 424 this.elemhideSelectorExceptions = new Map();
405 }; 425 };
406 426
407 /** 427 /**
408 * Add Adblock Plus filter to be converted 428 * Add Adblock Plus filter to be converted
409 * 429 *
410 * @param {Filter} filter Filter to convert 430 * @param {Filter} filter Filter to convert
411 */ 431 */
412 ContentBlockerList.prototype.addFilter = function(filter) 432 ContentBlockerList.prototype.addFilter = function(filter)
413 { 433 {
414 if (filter.sitekeys) 434 if (filter.sitekeys)
415 return; 435 return;
416 if (filter instanceof filterClasses.RegExpFilter && 436 if (filter instanceof filterClasses.RegExpFilter &&
417 filter.regexpSource == null) 437 filter.regexpSource == null)
418 return; 438 return;
419 439
420 if (filter instanceof filterClasses.BlockingFilter) 440 if (filter instanceof filterClasses.BlockingFilter)
421 this.requestFilters.push(filter); 441 this.requestFilters.push(filter);
422 442
423 if (filter instanceof filterClasses.WhitelistFilter) 443 if (filter instanceof filterClasses.WhitelistFilter)
424 { 444 {
425 if (filter.contentType & (typeMap.DOCUMENT | whitelistableRequestTypes)) 445 if (filter.contentType & (typeMap.DOCUMENT | whitelistableRequestTypes))
426 this.requestExceptions.push(filter); 446 this.requestExceptions.push(filter);
427 447
428 if (filter.contentType & typeMap.ELEMHIDE) 448 if (filter.contentType & typeMap.ELEMHIDE)
429 this.elemhideExceptions.push(filter); 449 this.elemhideExceptions.push(filter);
450 else if (filter.contentType & typeMap.GENERICHIDE)
451 this.generichideExceptions.push(filter);
430 } 452 }
431 453
432 if (filter instanceof filterClasses.ElemHideFilter) 454 if (filter instanceof filterClasses.ElemHideFilter)
433 this.elemhideFilters.push(filter); 455 this.elemhideFilters.push(filter);
434 456
435 if (filter instanceof filterClasses.ElemHideException) 457 if (filter instanceof filterClasses.ElemHideException)
436 { 458 {
437 let domains = this.elemhideSelectorExceptions[filter.selector]; 459 let domains = this.elemhideSelectorExceptions[filter.selector];
438 if (!domains) 460 if (!domains)
439 domains = this.elemhideSelectorExceptions[filter.selector] = []; 461 domains = this.elemhideSelectorExceptions[filter.selector] = [];
440 462
441 parseDomains(filter.domains, domains, []); 463 parseDomains(filter.domains, domains, []);
442 } 464 }
443 }; 465 };
444 466
445 /** 467 /**
446 * Generate content blocker list for all filters that were added 468 * Generate content blocker list for all filters that were added
447 * 469 *
448 * @returns {Filter} filter Filter to convert 470 * @returns {Filter} filter Filter to convert
449 */ 471 */
450 ContentBlockerList.prototype.generateRules = function(filter) 472 ContentBlockerList.prototype.generateRules = function(filter)
451 { 473 {
452 let rules = []; 474 let rules = [];
453 475
476 let genericSelectors = [];
454 let groupedElemhideFilters = new Map(); 477 let groupedElemhideFilters = new Map();
478
455 for (let filter of this.elemhideFilters) 479 for (let filter of this.elemhideFilters)
456 { 480 {
457 let result = convertElemHideFilter(filter, this.elemhideSelectorExceptions); 481 let result = convertElemHideFilter(filter, this.elemhideSelectorExceptions);
458 if (!result) 482 if (!result)
459 continue; 483 continue;
460 484
461 if (result.matchDomains.length == 0) 485 if (result.matchDomains.length == 0)
462 result.matchDomains = ["^https?://"]; 486 {
463 487 genericSelectors.push(result.selector);
464 for (let matchDomain of result.matchDomains) 488 }
465 { 489 else
466 let group = groupedElemhideFilters.get(matchDomain) || []; 490 {
467 group.push(result.selector); 491 for (let matchDomain of result.matchDomains)
468 groupedElemhideFilters.set(matchDomain, group); 492 {
469 } 493 let group = groupedElemhideFilters.get(matchDomain) || [];
470 } 494 group.push(result.selector);
495 groupedElemhideFilters.set(matchDomain, group);
496 }
497 }
498 }
499
500 addCSSRules(rules, genericSelectors, "^https?://");
501
502 // Right after the generic element hiding filters, add the exceptions that
503 // should apply only to those filters.
504 for (let filter of this.generichideExceptions)
505 convertFilterAddRules(rules, filter, "ignore-previous-rules", false);
471 506
472 groupedElemhideFilters.forEach((selectors, matchDomain) => 507 groupedElemhideFilters.forEach((selectors, matchDomain) =>
473 { 508 {
474 while (selectors.length) 509 addCSSRules(rules, selectors, matchDomain);
475 {
476 let selector = selectors.splice(0, selectorLimit).join(", ");
477
478 // As of Safari 9.0 element IDs are matched as lowercase. We work around
479 // this by converting to the attribute format [id="elementID"]
480 selector = convertIDSelectorsToAttributeSelectors(selector);
481
482 rules.push({
483 trigger: {"url-filter": matchDomain,
484 "url-filter-is-case-sensitive": true},
485 action: {type: "css-display-none",
486 selector: selector}
487 });
488 }
489 }); 510 });
490 511
491 for (let filter of this.elemhideExceptions) 512 for (let filter of this.elemhideExceptions)
492 convertFilterAddRules(rules, filter, "ignore-previous-rules", false); 513 convertFilterAddRules(rules, filter, "ignore-previous-rules", false);
493 for (let filter of this.requestFilters) 514 for (let filter of this.requestFilters)
494 convertFilterAddRules(rules, filter, "block", true); 515 convertFilterAddRules(rules, filter, "block", true);
495 for (let filter of this.requestExceptions) 516 for (let filter of this.requestExceptions)
496 convertFilterAddRules(rules, filter, "ignore-previous-rules", true); 517 convertFilterAddRules(rules, filter, "ignore-previous-rules", true);
497 518
498 return rules.filter(rule => !hasNonASCI(rule)); 519 return rules.filter(rule => !hasNonASCI(rule));
499 }; 520 };
LEFTRIGHT

Powered by Google App Engine
This is Rietveld