Issue 5315 - Add support for Microsoft Edge

options.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2017 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 515 matching lines @@
   let elt = new Option();
   elt.text = text;
   elt.value = text;
   document.getElementById(boxId).appendChild(elt);
 }
 
 // Remove a filter string from a list box.
 function removeFromListBox(boxId, text)
 {
   let list = document.getElementById(boxId);
-  let selector = "option[value=" + CSS.escape(text) + "]";
-  for (let option of list.querySelectorAll(selector))
-    list.removeChild(option);
+  // Edge does not support CSS.escape yet:
+  // https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/101410/
+  if (typeof CSS.escape == "undefined")
+  {
+    for (let i = 0; i < list.length; i++)

[kzar, 2017/07/17 14:08:19]

This will be kinda slow when there are lots of filters, but I don't have a
better idea other than attempting to reimplement CSS.escape.

Any reason you didn't use for ... of? I imagined it something like this
(untested):

for (let option of list.options)
{
  if (option.value == text)
    list.removeChild(option);
}

[Sebastian Noack, 2017/07/17 14:17:21]

Couldn't we just do this instead of CSS.escape():

  var selector = "option[value='" + text.replace(/'/g, "\\'") + "']";

Of course using CSS.escape() would be better, but in this case I think it would
be more preferable to have the same code path on all platforms.

[kzar, 2017/07/17 14:27:02]

I'm not sure that would always work, for example what if the string contains
"\'", wouldn't it become "\\'"? But I do agree if we can find a reasonably
simple way to always use the same code path here it would be nice, and using
querySelectorAll here will be much faster for large numbers of filters.

[Sebastian Noack, 2017/07/17 14:31:25]

It seems that we have already implemented the same logic in a feature complete
and reliable way that doesn't rely on CSS.escape() here:
https://hg.adblockplus.org/adblockpluschrome/file/efc63b673869/lib/filterComp...

Perhaps we can just reuse that code here (ideally without duplication).

[kzar, 2017/07/17 14:41:37]

Sounds good to me.

Though to avoid duplicating the logic the only (non-hack) way I can think of
doing it would be to have filterComposer.js combined with options.js at build
time so that require("filterComposer").quoteCSS would work here. But that seems
like kind of overkill, what do you think?

[Sebastian Noack, 2017/07/17 15:49:56]

How about using messaging?

[Oleksandr, 2017/07/17 23:52:09]

Done.

+      if (list.options[i].value == text)
+        list.remove(i--);
+  }
+  else
+  {
+    let selector = "option[value=" + CSS.escape(text) + "]";
+    for (let option of list.querySelectorAll(selector))
+      list.removeChild(option);
+  }
 }
 
 function addWhitelistDomain(event)
 {
   event.preventDefault();
 
   let domain = document.getElementById(
     "newWhitelistDomain"
   ).value.replace(/\s/g, "");
   document.getElementById("newWhitelistDomain").value = "";
@@ skipping 184 matching lines @@
       onFilterMessage(message.action, message.args[0]);
       break;
     case "prefs.respond":
       onPrefMessage(message.action, message.args[0]);
       break;
     case "subscriptions.respond":
       onSubscriptionMessage(message.action, message.args[0]);
       break;
   }
 });