Closed Bug 1073415 Opened 5 years ago Closed 5 years ago

Empty Loop landing page on Internet Explorer 11 for link clickers

Categories

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

All
Windows 8
defect
Points:
3

Tracking

(firefox34 affected, firefox35 fixed, firefox36 fixed)

VERIFIED FIXED
mozilla36
Iteration:
36.2
Tracking Status
firefox34 --- affected
firefox35 --- fixed
firefox36 --- fixed
Blocking Flags:
backlog Fx34+

People

(Reporter: RT, Assigned: jaws)

References

Details

(Whiteboard: [standalone][todo: see user story])

User Story

Todo:

1) File bugs/discuss with Gaia folks about landing patches on gaia's l10n file
2) Land the Loop specific polyfills in a nice way
3) If 1 takes more than a day or so, then consider landing the patches specific to loop in loop, with a readme or other indication that its a patched gaia file - to unblock getting this bug fixed.

Attachments

(1 file, 4 obsolete files)

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
Translated error:

SCRIPT5007: Unable to get property 'noCompleteBug' of undefined or null reference 
l10n-gaia-02ca67948fe8.js, line 1462 character 5
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
Summary: Empty Loop landing page on IE for link clickers → Empty Loop landing page on Internet Explorer (IE) for link clickers
Assignee: nobody → standard8
Iteration: --- → 36.1
Points: --- → 3
Hardware: x86_64 → All
Target Milestone: --- → mozilla36
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.
Duplicate of this bug: 1064173
backlog: --- → Fx34+
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.
Attachment #8506924 - Flags: feedback?(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.
Attachment #8506924 - Flags: feedback?(dmose)
Whiteboard: [standalone]
Duplicate of this bug: 1086928
Target Milestone: mozilla36 → ---
(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.
Assignee: standard8 → nobody
User Story: (updated)
Whiteboard: [standalone] → [standalone][todo: see user story]
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.
Summary: Empty Loop landing page on Internet Explorer (IE) for link clickers → Empty Loop landing page on Internet Explorer (IE 10 & 11) for link clickers
Flags: qe-verify+
Flags: firefox-backlog+
Assignee: nobody → jaws
Iteration: 36.1 → 36.2
Status: NEW → ASSIGNED
Priority: -- → P1
Iteration: 36.2 → ---
Iteration: --- → 36.2
Duplicate of this bug: 1082941
Note, if the TB plugin is installed then IE link clickers should be able to make their call.
(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.
Flags: needinfo?(rtestard)
Agreed this is separate.
I created the bug tracking IE plug-in support - bug 1091537
Flags: needinfo?(rtestard)
Attached patch Patch (obsolete) — Splinter Review
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.
Attachment #8504897 - Attachment is obsolete: true
Attachment #8506924 - Attachment is obsolete: true
Attachment #8517648 - Flags: review?(dmose)
Attached patch Patch v1.1 (obsolete) — Splinter Review
Now also providing a dummy MutationObserver. Wasn't noticed before as I was testing with IE11 and MutationObservers are defined starting in IE11.
Attachment #8517648 - Attachment is obsolete: true
Attachment #8517648 - Flags: review?(dmose)
Attachment #8517737 - Flags: review?(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.
Attachment #8517737 - Flags: review?(standard8) → review?(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.
Attachment #8517737 - Flags: review?(dmose) → review+
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.
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.
Flags: in-moztrap?(anthony.s.hughes)
(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.
Attached patch Patch w/ r+Splinter Review
Attachment #8517737 - Attachment is obsolete: true
Attachment #8518499 - Flags: review+
https://hg.mozilla.org/integration/fx-team/rev/94c91e56fc3b
Whiteboard: [standalone][todo: see user story] → [standalone][todo: see user story][fixed-in-fx-team]
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.
Flags: in-moztrap?(anthony.s.hughes) → in-moztrap+
https://hg.mozilla.org/mozilla-central/rev/94c91e56fc3b
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [standalone][todo: see user story][fixed-in-fx-team] → [standalone][todo: see user story]
Target Milestone: --- → mozilla36
Paul, can you please verify this is fixed tomorrow?
Flags: needinfo?(paul.silaghi)
QA Contact: anthony.s.hughes → paul.silaghi
(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.
Flags: needinfo?(paul.silaghi) → needinfo?(jaws)
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.
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 on attachment 8518499 [details] [diff] [review]
Patch w/ r+

Approval Request Comment
Landed on aurora per IRC with lsblakk with a=loop-only
Attachment #8518499 - Flags: approval-mozilla-aurora?
Attachment #8518499 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(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.
Depends on: 1097852
Flags: needinfo?(jaws)
Summary: Empty Loop landing page on Internet Explorer (IE 10 & 11) for link clickers → Empty Loop landing page on Internet Explorer 11 for link clickers
Marking this verified based on comment 26.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.