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

Delta Between Two Patch Sets: lib/elemHide.js

Issue 30002601: Issue 7284 - Move CSS escaping to createStyleSheet (Closed) Base URL: https://hg.adblockplus.org/adblockpluscore/
Left Patch Set: Reworked the escaping of selectors Created Feb. 12, 2019, 11:50 p.m.
Right Patch Set: more nits Created Feb. 18, 2019, 1:50 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 | « lib/content/elemHideEmulation.js ('k') | lib/filterClasses.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-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
(...skipping 459 matching lines...) Expand 10 before | Expand all | Expand 10 after
470 // calculate the sizes of the selectors and divide them up accordingly, but 470 // calculate the sizes of the selectors and divide them up accordingly, but
471 // this approach is more efficient and has worked well in practice. In theory 471 // this approach is more efficient and has worked well in practice. In theory
472 // this could still lead to some selectors not working on Chromium, but it is 472 // this could still lead to some selectors not working on Chromium, but it is
473 // highly unlikely. 473 // highly unlikely.
474 // See issue #6298 and https://crbug.com/804179 474 // See issue #6298 and https://crbug.com/804179
475 for (let i = 0; i < selectors.length; i += selectorGroupSize) 475 for (let i = 0; i < selectors.length; i += selectorGroupSize)
476 yield selectors.slice(i, i + selectorGroupSize); 476 yield selectors.slice(i, i + selectorGroupSize);
477 } 477 }
478 478
479 /** 479 /**
480 * Escape curly braces to prevent CSS rule injection. 480 * Escapes curly braces to prevent CSS rule injection.
Manish Jethani 2019/02/13 12:27:25 s/Escape/Escapes/
hub 2019/02/13 16:33:32 Done.
481 * 481 *
482 * @param {string} selector 482 * @param {string} selector
483 * @return {string} 483 * @returns {string}
Manish Jethani 2019/02/13 12:27:25 s/@return/@returns/
hub 2019/02/13 16:33:31 Done.
484 */ 484 */
485 function escapedSelector(selector) 485 function escapeSelector(selector)
hub 2019/02/12 23:55:49 while we expect a string, if `selector` is undefin
Manish Jethani 2019/02/13 12:27:25 If it's always going to be a string in practice th
Manish Jethani 2019/02/13 12:27:25 Let's rename to `escapeSelector`
hub 2019/02/13 16:33:32 Done.
486 { 486 {
487 return selector.replace("{", "\\7B ").replace("}", "\\7D "); 487 return selector.replace("{", "\\7B ").replace("}", "\\7D ");
488 } 488 }
489 489
490 /** 490 /**
491 * Creates an element hiding CSS rule for a given list of selectors. 491 * Creates an element hiding CSS rule for a given list of selectors.
492 * 492 *
493 * @param {Array.<string>} selectors 493 * @param {Array.<string>} selectors
494 * @returns {string} 494 * @returns {string}
495 */ 495 */
496 function createRule(selectors) 496 function createRule(selectors)
497 { 497 {
498 let rule = ""; 498 let rule = "";
499 499
500 for (let i = 0; i < selectors.length - 1; i++) 500 for (let i = 0; i < selectors.length - 1; i++)
501 rule += escapedSelector(selectors[i]) + ", "; 501 rule += escapeSelector(selectors[i]) + ", ";
502 502
503 rule += escapedSelector(selectors[selectors.length - 1]) + 503 rule += escapeSelector(selectors[selectors.length - 1]) +
504 " {display: none !important;}\n"; 504 " {display: none !important;}\n";
Manish Jethani 2019/02/13 12:27:25 Nit: The double quote here should be in the same c
hub 2019/02/13 16:33:32 Done.
505 505
506 return rule; 506 return rule;
507 } 507 }
508 508
509 /** 509 /**
510 * Creates an element hiding CSS style sheet from a given list of selectors. 510 * Creates an element hiding CSS style sheet from a given list of selectors.
511 * @param {Array.<string>} selectors 511 * @param {Array.<string>} selectors
512 * @returns {string} 512 * @returns {string}
513 */ 513 */
514 function createStyleSheet(selectors) 514 function createStyleSheet(selectors)
515 { 515 {
516 let styleSheet = ""; 516 let styleSheet = "";
517 517
518 for (let selectorGroup of splitSelectors(selectors)) 518 for (let selectorGroup of splitSelectors(selectors))
519 styleSheet += createRule(selectorGroup); 519 styleSheet += createRule(selectorGroup);
520 520
521 return styleSheet; 521 return styleSheet;
522 } 522 }
523 523
524 exports.createStyleSheet = createStyleSheet; 524 exports.createStyleSheet = createStyleSheet;
LEFTRIGHT

Powered by Google App Engine
This is Rietveld