Issue 1587 - Escape curly brackets for elemhide filters

qunit/tests/cssEscaping.js

Index: qunit/tests/cssEscaping.js
===================================================================
--- a/qunit/tests/cssEscaping.js
+++ b/qunit/tests/cssEscaping.js
@@ -18,6 +18,8 @@
       }.bind(this));
 
       document.body.appendChild(frame);
+
+      this.filterClasses = require("filterClasses");

[Wladimir Palant, 2014/11/20 14:33:05]

As with the other review, no point having this in the test setup - you can
require the module at the top of the file.

[Sebastian Noack, 2014/11/20 15:47:27]

Done, using an IFEE.

     }
   }
 );
@@ -27,6 +29,9 @@
   var escapeCSS = this.escapeCSS;
   var quote = this.quote;
 
+  var Filter = this.filterClasses.Filter;
+  var ElemHideFilter = this.filterClasses.ElemHideFilter;
+
   function testSelector(opts)
   {
     var mustMatch = opts.mustMatch !== false;
@@ -59,10 +64,19 @@
     }
 
     var foundElement = doc.querySelector(opts.selector);
-    if (mustMatch)
-      equal(foundElement, element, opts.selector);
+    var filter = Filter.fromText("##" + opts.selector);
+
+    if (!(filter instanceof ElemHideFilter))
+    {
+      ok(false, opts.selector + " (not allowed in elemhide filters)");

[Wladimir Palant, 2014/11/20 14:33:05]

There are very few cases where code like this is justified:

  if (foo)
    ok(false);

Normally, you just check foo with qUnit:

  ok(filter instanceof ElemHideFilter, opts.selector + " allows in elemhide
filters");

[Sebastian Noack, 2014/11/20 15:47:27]

So far testSelector() only generates one assert per call. And since it is called
1540 times I prefer to keep it that way. All you need to know is whether the
selector is valid CSS, and whether it is compatible with elemhide filters. And
the message we log here provides those information.

[Wladimir Palant, 2014/11/20 17:01:20]

Not sure I agree but not really that important...

+    }
     else
-      ok(true, opts.selector);
+    {
+      if (mustMatch)
+        equal(foundElement, element, opts.selector);
+      else
+        ok(true, opts.selector);
+    }
   }
 
   function testEscape(s)