Issue 7284 - Move CSS escaping to createStyleSheet

lib/elemHide.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 459 matching lines @@
   // calculate the sizes of the selectors and divide them up accordingly, but
   // this approach is more efficient and has worked well in practice. In theory
   // this could still lead to some selectors not working on Chromium, but it is
   // highly unlikely.
   // See issue #6298 and https://crbug.com/804179
   for (let i = 0; i < selectors.length; i += selectorGroupSize)
     yield selectors.slice(i, i + selectorGroupSize);
 }
 
 /**
- * Escape 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.

+ * Escapes curly braces to prevent CSS rule injection.
  *
  * @param {string} selector
- * @return {string}

[Manish Jethani, 2019/02/13 12:27:25]

s/@return/@returns/

[hub, 2019/02/13 16:33:31]

Done.

- */
-function escapedSelector(selector)

[hub, 2019/02/12 23:55:49]

while we expect a string, if `selector` is undefined it will throw an error. The
bug in test I fixed was triggering it.
So I'm pondering if we shouldn't check for undefined (and null?).

[Manish Jethani, 2019/02/13 12:27:25]

If it's always going to be a string in practice then we should not check for
null. The tests should not try to pass a non-string.

[Manish Jethani, 2019/02/13 12:27:25]

Let's rename to `escapeSelector`

[hub, 2019/02/13 16:33:32]

Done.

+ * @returns {string}
+ */
+function escapeSelector(selector)
 {
   return selector.replace("{", "\\7B ").replace("}", "\\7D ");
 }
 
 /**
  * Creates an element hiding CSS rule for a given list of selectors.
  *
  * @param {Array.<string>} selectors
  * @returns {string}
  */
 function createRule(selectors)
 {
   let rule = "";
 
   for (let i = 0; i < selectors.length - 1; i++)
-    rule += escapedSelector(selectors[i]) + ", ";
-
-  rule += escapedSelector(selectors[selectors.length - 1]) +
-    " {display: none !important;}\n";

[Manish Jethani, 2019/02/13 12:27:25]

Nit: The double quote here should be in the same column as the 'e' of
'escapedSelector' above.

[hub, 2019/02/13 16:33:32]

Done.

+    rule += escapeSelector(selectors[i]) + ", ";
+
+  rule += escapeSelector(selectors[selectors.length - 1]) +
+          " {display: none !important;}\n";
 
   return rule;
 }
 
 /**
  * Creates an element hiding CSS style sheet from a given list of selectors.
  * @param {Array.<string>} selectors
  * @returns {string}
  */
 function createStyleSheet(selectors)
 {
   let styleSheet = "";
 
   for (let selectorGroup of splitSelectors(selectors))
     styleSheet += createRule(selectorGroup);
 
   return styleSheet;
 }
 
 exports.createStyleSheet = createStyleSheet;