Issue 4698 - Popup window is broken in Edge

Issue 4698 - Popup window is broken in Edge

[Oleksandr, 2016-12-05 00:27:36]

https://codereview.adblockplus.org/29366773/diff/29366774/stats.js
File stats.js (right):

https://codereview.adblockplus.org/29366773/diff/29366774/stats.js#newcode99
stats.js:99: i18n.setElementText(statsPage, "stats_label_page", blockedPage);
Edge throws if passed an array as a second parameter to i18n.getMessage. Since
passing a single element is supported in Chrome as well, this looks like a safe
workaround.

[kzar, 2016-12-05 12:22:52]

https://codereview.adblockplus.org/29366773/diff/29366774/stats.js
File stats.js (right):

https://codereview.adblockplus.org/29366773/diff/29366774/stats.js#newcode99
stats.js:99: i18n.setElementText(statsPage, "stats_label_page", blockedPage);
On 2016/12/05 00:27:36, Oleksandr wrote:
> Edge throws if passed an array as a second parameter to i18n.getMessage. Since
> passing a single element is supported in Chrome as well, this looks like a
safe
> workaround.

How come that throws, since according to the Mozilla docs[1] it's supported?
What's the exception that's thrown? (I wonder how you're supposed to translate a
string with multiple substitutions if passing an array of substitutions doesn't
work.)

If this is a bug in the Edge extension API then please could you file an issue
with them?

[1]
https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/i18n/getMessage

[Oleksandr, 2016-12-05 14:48:18]

On 2016/12/05 12:22:52, kzar wrote:
> https://codereview.adblockplus.org/29366773/diff/29366774/stats.js
> File stats.js (right):
> 
> https://codereview.adblockplus.org/29366773/diff/29366774/stats.js#newcode99
> stats.js:99: i18n.setElementText(statsPage, "stats_label_page", blockedPage);
> On 2016/12/05 00:27:36, Oleksandr wrote:
> > Edge throws if passed an array as a second parameter to i18n.getMessage.
Since
> > passing a single element is supported in Chrome as well, this looks like a
> safe
> > workaround.
> 
> How come that throws, since according to the Mozilla docs[1] it's supported?
> What's the exception that's thrown? (I wonder how you're supposed to translate
a
> string with multiple substitutions if passing an array of substitutions
doesn't
> work.)
> 
> If this is a bug in the Edge extension API then please could you file an issue
> with them?
> 
> [1]
> https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/i18n/getMessage

It looks like it is a problem only if you call getMessage from a contentScript.
I have filed a Microsoft Edge bug here: 
https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/10063392/

[kzar, 2016-12-05 15:46:10]

On 2016/12/05 14:48:18, Oleksandr wrote:
> It looks like it is a problem only if you call getMessage from a
contentScript.
> I have filed a Microsoft Edge bug here: 
> https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/10063392/

Thanks for filing the Edge issue, I added a link to it from our issue as well.
BTW please reply to inline comments inline, otherwise it's hard to keep a
conversation going.

Assuming this change is for the edge branch then LGTM