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

Unified Diff: options.js

Issue 29458601: Issue 5315 - Add support for Microsoft Edge (Closed)
Patch Set: Workaround CSS.excape, i18n issue and use typeof for 'undefined' detection Created July 17, 2017, 12:51 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« ext/popup.js ('K') | « metadata.edge ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: options.js
===================================================================
--- a/options.js
+++ b/options.js
@@ -533,9 +533,20 @@
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 fil
Sebastian Noack 2017/07/17 14:17:21 Couldn't we just do this instead of CSS.escape():
kzar 2017/07/17 14:27:02 I'm not sure that would always work, for example w
Sebastian Noack 2017/07/17 14:31:25 It seems that we have already implemented the same
kzar 2017/07/17 14:41:37 Sounds good to me. Though to avoid duplicating th
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)
« ext/popup.js ('K') | « metadata.edge ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld