Issue 6868 - Enable capturing of surrounding wildcards for rewrite filters

lib/common.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 14 matching lines @@
 function textToRegExp(text)
 {
   return text.replace(/[-/\\^$*+?.()|[\]{}]/g, "\\$&");
 }
 
 exports.textToRegExp = textToRegExp;
 
 /**
  * Converts filter text into regular expression string
  * @param {string} text as in Filter()
- * @param {boolean} [optimize=true] whether to optimize the regular expression
+ * @param {boolean} [captureAll=false] whether to enable the capturing of
+ *   leading and trailing wildcards in the filter text; by default, leading and
+ *   trailing wildcards are stripped out
  * @return {string} regular expression representation of filter text
  */
-function filterToRegExp(text, optimize = true)

[hub, 2018/08/29 16:23:50]

also, on second thought, I'm not certain that `optimize` is the best naming for
this option. Maybe "capture"? Since the use case is to capture what the regexp
match.

[Manish Jethani, 2018/09/01 14:57:52]

OK, it already captures everything else, just not the stuff that's leading and
trailing. How about `captureAll` in that case?

-{
-  if (optimize)
-  {
-    // remove multiple wildcards
-    text = text.replace(/\*+/g, "*");

[hub, 2018/08/28 22:02:55]

I think we should keep this one unconditionally like it was before, but run it
before this block.

[Manish Jethani, 2018/08/29 13:25:24]

Since the `optimize` argument is supposed to skip all optimizations, shouldn't
we skip this one too? It doesn't make any difference to the matching behavior of
the regular expression, it only makes it match faster, and one can argue that
it's not important for rewrite filters (why would anyone even use two or more
consecutive "*").

[hub, 2018/08/29 16:23:50]

this is more about correctness IMHO. There is an expectation to remove duplicate
'*'

[Manish Jethani, 2018/09/01 14:57:52]

Alright, if we go with the renaming then it makes sense.

Done.

-
+function filterToRegExp(text, captureAll = false)
+{
+  // remove multiple wildcards
+  text = text.replace(/\*+/g, "*");
+
+  if (!captureAll)
+  {
     // remove leading wildcard
     if (text[0] == "*")
       text = text.substring(1);
 
     // remove trailing wildcard
     if (text[text.length - 1] == "*")
       text = text.substring(0, text.length - 1);
   }
 
   return text
@@ skipping 150 matching lines @@
     // Note that the first group in the regular expression is optional. If it
     // doesn't match (e.g. "#foo::nth-child(1)"), type will be an empty string.
     qualifiedSelector += sub.substr(0, index) + type + qualifier + rest;
   }
 
   // Remove the initial comma and space.
   return qualifiedSelector.substr(2);
 }
 
 exports.qualifySelector = qualifySelector;