1412741 - Hanging when signing into IFTTT via Google

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

- Signed into a Google account already
- Visit https://ifttt.com/login
- Click "Continue with Google"
- Select the signed in account

Expected: sign in
Actual: dialog closes and the browser starts to hang

This is 57.0b12 on macOS Sierra. I don't know if this affects other platforms.

Comment 1

[Vlad Bacia-Mociran [:VladB]]

User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20171030103605

I have tested your issue on latest Nightly build (Buid ID: 20171030103605) and managed to reproduce it. I have signed into a google account on Gmail and after that I have opened https://ifttt.com/login page in a new tab and signed in with the Google account. The login pop-up window is dismissed but the browser tab hangs.

I did a regression range for the issue and this is the output:
INFO: Narrowed inbound regression window from [d649fe4b, c99d20ba] (3 builds) to [c91796e4, c99d20ba] (2 builds) (~1 steps left)
INFO: No more inbound revisions, bisection finished.
INFO: Last good revision: c91796e4b8bdb6f60c5ae5fc5559e1243d0e42cb
INFO: First bad revision: c99d20ba408a4fd19bab0bc04ff926e73d838fbf
INFO: Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c91796e4b8bdb6f60c5ae5fc5559e1243d0e42cb&tochange=c99d20ba408a4fd19bab0bc04ff926e73d838fbf

@Boris, can you take a look at this?

Comment 2

[Boris Zbarsky [:bzbarsky]]

This is a regression from bug 1381408.

We're ending up in infinite recursion in this function (defined in <https://web-assets.ifttt.com/assets/application-85c1d11cdbad117974cfda954ed63f17569e1e6c21ab2ef5459d1a39fa72fa28.js>) on the page:

m.extend = m.fn.extend = function() {
                    var e, t, n, r, o, i, a = arguments[0] || {},
                        s = 1,
                        u = arguments.length,
                        l = !1;
                    for ("boolean" == typeof a && (l = a, a = arguments[s] || {}, s++), "object" == typeof a || m.isFunction(a) || (a = {}), s === u && (a = this, s--); u > s; s++)
                        if (null != (e = arguments[s]))
                            for (t in e) n = a[t], r = e[t], a !== r && (l && r && (m.isPlainObject(r) || (o = Array.isArray(r))) ? (o ? (o = !1, i = n && Array.isArray(n) ? n : []) : i = n && m.isPlainObject(n) ? n : {}, a[t] = m.extend(l, i, r)) : void 0 !== r && (a[t] = r));
                    return a
                }

In a debugger I see us keep enumerating a property named "window", which is the first cross-origin-readable thing defined on Window.webidl.  Note that a cross-origin object would test true for isPlainObject in that script, afaict.  The key bits are:

   for (t in e) n = a[t]

and

   i = n && m.isPlainObject(n) ? n : {}

and

   a[t] = m.extend(l, i, r))

So as far as I can tell, once this function gets its hands on a cross-origin window it will think that it's a plain object and it should recursively call "extend" on it, which will just land right back in this function with the exact same object.

Of course this function would also fail on any plain-object object graph with loops in it... but they're not passing those in.

It looks to me like this function is actually jQuery.extend as documented at http://api.jquery.com/jQuery.extend/ (at least the overall structure matches what I see in https://code.jquery.com/jquery-3.2.1.js modulo minification).

Given that, I think we should back out bug 1381408 and push back on the spec change.

Comment 3

[Boris Zbarsky [:bzbarsky]]

I filed a spec issue at https://github.com/whatwg/html/issues/3183

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Back out the fix for bug 1381408, because that change doesn't look like it's web-compatible

MozReview-Commit-ID: LSYYAt0bAkT

Comment 5

[Peter Van der Beken [:peterv]]

Comment on attachment 8924054 [details] [diff] [review]
Back out the fix for bug 1381408, because that change doesn't look like it's web-compatible

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

Sigh.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1fdce3a0396c
Back out the fix for bug 1381408, because that change doesn't look like it's web-compatible.  r=peterv

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8924054 [details] [diff] [review]
Back out the fix for bug 1381408, because that change doesn't look like it's web-compatible

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1381408
[User impact if declined]: Hangs or incorrect functionality on some sites.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Not yet.
[Needs manual test from QE? If yes, steps to reproduce]: See comment 0.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: Not particularly.
[Why is the change risky/not risky?]: This is just reverting to the Firefox 55
   behavior, which also matches other browsers.
[String changes made/needed]: None.

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/1fdce3a0396c

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8924054 [details] [diff] [review]
Back out the fix for bug 1381408, because that change doesn't look like it's web-compatible

Backout, reverts to 57 behavior, Beta57+

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/4288e0439f90 (FIREFOX_57b_RELBRANCH)
https://hg.mozilla.org/releases/mozilla-release/rev/ec8faf9d7f3e

Comment 11

[Hani Yacoub, Desktop QA]

Build ID: 20171114100042
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:59.0) Gecko/20100101 Firefox/59.0

Verified as fixed on Firefox Nightly 59.0a1 on Windows 10 x 64, Windows 7 x32, Mac OS X 10.12 and Ubuntu 16.04 x64.

Comment 12

[Hani Yacoub, Desktop QA]

Verified as fixed on Firefox 57.0 and Firefox 58.0b3 on Windows 10 x 64, Windows 7 x32, Mac OS X 10.12 and Ubuntu 16.04 x64.