1073415 - Empty Loop landing page on Internet Explorer 11 for link clickers

Status: VERIFIED FIXED

(Hello (Loop) :: Client, defect, P1)

Description

[Romain Testard [:RT]]

When reaching a Loop URL on IE, the result is an empty page.
The expectation is that the user gets informed that his browser is not supported.

JS log:
SCRIPT5007: Impossible d’obtenir la propriété  « noCompleteBug » d’une référence null ou non définie 
l10n-gaia-02ca67948fe8.js, Ligne 1462 Caractère 5

Comment 1

[Mark Banner (:standard8)]

Translated error:

SCRIPT5007: Unable to get property 'noCompleteBug' of undefined or null reference 
l10n-gaia-02ca67948fe8.js, line 1462 character 5

Comment 2

[Romain Testard [:RT]]

The above was for IE 10.0.9200.17088 on Windows 8

Errors details using IE 11.0.9600.17280 on Windows 7:
File: call.mozilla.com
SCRIPT1028: Identifier, string or number unexpected
File: call.mozilla.com, line: 26, column : 7
SCRIPT1003: ':' expected
File: l10n-gaia-02ca67948fe8.js, line: 1388, column: 9
SCRIPT438: The object can't manage the property or method « isArray »
File: react-0.11.1.js, line: 18991, column: 37

Comment 3

[Mark Banner (:standard8)]

Attachment: Initial hacks for getting the page to work in IE

This are some initial hacks I've done for getting it to work in IE. There's a couple of polyfills, for things IE doesn't/didn't support.

IE also doesn't support MutationObservers, as we don't really need those, I've just disabled them for now.

There's also one other thing, which looks like a javascript strict issue.

Next steps are to tidy this up a bit and propose some of the changes to the upstream folks in gaia land.

Comment 5

[Mark Banner (:standard8)]

Attachment: Add some polyfills for supporting IE for Loop's use of the l10n-gaia library

This separates out the polyfill work from the gaia specific parts of the patch. Looking for feedback at this stage, until I get the main part of the patch up and ready.

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 8506924 [details] [diff] [review]
Add some polyfills for supporting IE for Loop's use of the l10n-gaia library

Review of attachment 8506924 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/loop/standalone/content/index.html
@@ +35,5 @@
> +      }
> +      // To support IE for the l10n-gaia library.
> +     if (!"language" in navigator) {
> +        navigator.language = navigator.browserLanguage;
> +      }

My primary concern with this patch is that it might do something weird in an older browser that has neither window.CustomEvent nor support for Event.initCustomEvent.  I don't know whether there are many such browsers in the field these days.

Of course, presumably we'd already be broken on such browsers, so this wouldn't make things worse.

Comment 8

[Mark Banner (:standard8)]

(In reply to Dan Mosedale (:dmose) - not reading bugmail; needinfo? for response from comment #6)
> My primary concern with this patch is that it might do something weird in an
> older browser that has neither window.CustomEvent nor support for
> Event.initCustomEvent.  I don't know whether there are many such browsers in
> the field these days.

The polyfill here was recommended on devmo, so I'd be surprised if there's major issues with it.

> Of course, presumably we'd already be broken on such browsers, so this
> wouldn't make things worse.

For now, I think this is better than what we've got, so its worth moving forward.

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

Given what MDN says about this bug, jaws and I suspect it'll be straightforward to reuse Mark's work on IE 9 and later.

However, we presumably also want it working on Windows XP, since there's a non-trivial number of users there.  Since we don't know what else is going to be missing in IE8 (latest browser released on XP), we'll estimate for IE9 and later.  If that turns out to be a non-trivial amount of work to extend into IE7, then we'll spin out into another bug for that.

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

Note, if the TB plugin is installed then IE link clickers should be able to make their call.

Comment 12

[Mark Banner (:standard8)]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #11)
> Note, if the TB plugin is installed then IE link clickers should be able to
> make their call.

We should deal with that totally separately - there's different UI to be displayed (the TB UI isn't localised) and things to set up. I believe there's a bug around somewhere, but I can't find it at the moment - hopefully RT will know.

Comment 13

[Romain Testard [:RT]]

Agreed this is separate.
I created the bug tracking IE plug-in support - bug 1091537

Comment 14

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

This gets the link-clicker landing page to display for me. I had to switch the polyfill around based on how IE11 behaves (noted in a comment).

multiplexGum has an issue with the TBPlugin, since it checks for TBPlugin but that object is not defined until the sdk is loaded (which needs to be after multiplexGum). With that being said, that is not part of this bug, so not addressed in this patch.

Comment 15

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch v1.1

Now also providing a dummy MutationObserver. Wasn't noticed before as I was testing with IE11 and MutationObservers are defined starting in IE11.

Comment 16

[Mark Banner (:standard8)]

Comment on attachment 8517737 [details] [diff] [review]
Patch v1.1

Dan, can you review this? I'm primarily focusing on rooms at the moment.

Comment 17

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 8517737 [details] [diff] [review]
Patch v1.1

Review of attachment 8517737 [details] [diff] [review]:
-----------------------------------------------------------------

r=dmose with comments addressed.

::: browser/components/loop/standalone/content/index.html
@@ +24,5 @@
> +      // window.CustomEvent object, but it throws when new CustomEvent(...) is
> +      // called, with the exception of "Object doesn't support this action".
> +      try {
> +        if (window.CustomEvent) {
> +          var customEventCtor = new CustomEvent("test", {"detail":{"sampleProperty":true}});

If you could hoist the var just code readability, that'd be great.  Maybe rename the var workingCustomEventCtor?

@@ +43,5 @@
> +      // To support IE for the l10n-gaia library.
> +      if (!"language" in navigator) {
> +        navigator.language = navigator.browserLanguage;
> +      }
> +      if(!window.MutationObserver) {

Whitespace after if and before parens, please.  Also, please add a comment about which versions of what browsers this is here for so that we know when we can consider removing it.

Comment 18

[Dan Mosedale (:dmosedale, :dmose)]

Please also either add a moztrap test case for this, or set the approriate qa flag to cause that to happen with a description of the tests.

Comment 19

[Jared Wein [:jaws] (please needinfo? me)]

Anthony, for the moztrap test, we want to attempt an end-to-end call, with the link clicker (caller) being on IE9, IE10, and IE11. It does not matter what version of Firefox the callee is using.

We'd like this moztrap test to be re-run every time a new Windows version, service pack, or IE version is released.

Comment 20

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #19)
> Anthony, for the moztrap test, we want to attempt an end-to-end call, with
> the link clicker (caller) being on IE9, IE10, and IE11. It does not matter
> what version of Firefox the callee is using.
> 
> We'd like this moztrap test to be re-run every time a new Windows version,
> service pack, or IE version is released.

Also, the tester will should test with and without the Tokbox IE plugin installed.

Comment 21

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch w/ r+

Comment 22

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/integration/fx-team/rev/94c91e56fc3b

Comment 23

[u279076]

I already have a Moztrap test:
https://moztrap.mozilla.org/manage/case/14509/

While it is not specific to this bug it should be good enough to cover this issue.

Comment 24

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/94c91e56fc3b

Comment 25

[u279076]

Paul, can you please verify this is fixed tomorrow?

Comment 26

[Paul Silaghi, QA [:pauly]]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #19)
> Anthony, for the moztrap test, we want to attempt an end-to-end call, with
> the link clicker (caller) being on IE9, IE10, and IE11. It does not matter
> what version of Firefox the callee is using.
Works fine in IE11.
Doesn't work in IE9, 10 -> empty page.
Tested on Win 7 x64, 36.0a1 (2014-11-09) using the dev-server.

Comment 27

[Dan Mosedale (:dmosedale, :dmose)]

Rather than trying to fix this, if it's easy, we could consider experimenting with <https://hacks.mozilla.org/2014/11/an-easier-way-of-using-polyfills/>. Of course, if doing such an experiment isn't super easy, now's the wrong for that.

Comment 28

[u279076]

How do you want to handle this? Should we rename this bug to be just about IE11 and call it done then file a new bug for IE9/10?

Comment 29

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e7ba3a80159b

Comment 30

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8518499 [details] [diff] [review]
Patch w/ r+

Approval Request Comment
Landed on aurora per IRC with lsblakk with a=loop-only

Comment 31

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #28)
> How do you want to handle this? Should we rename this bug to be just about
> IE11 and call it done then file a new bug for IE9/10?

Yeah, I filed a new bug for IE9 and IE10 to figure out what is different there. Filed as bug 1097852.

Comment 32

[Paul Silaghi, QA [:pauly]]

Marking this verified based on comment 26.