1012944 - User login and account creation on deezer.com broken since Firefox 30.0b1, say home.display is not a function

Status: VERIFIED FIXED

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

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Yesterday I have noticed with my Aurora build that I cannot log into deezer.com anymore. I had to do that, given that I accidentally removed my cookies in the given Firefox profile. I quickly checked which versions of Firefox are affected, and it goes back to 30.0b1. Firefox 29.0.1 is not affected.

Steps:
1. Go to https://www.deezer.com/?lb=login
2. Login if you already have an account, or create a new account
3. Submit your entered data 

After step 3 you will be redirected to the start page. There is no way to create an account or to login.

I will do a regression test tomorrow, if no-one else got already around it.

Comment 1

[Kohei Yoshino [:kohei]]

The console says TypeError: home.display is not a function

https://cdns-files.deezer.com/js/min/home-v00206824.js

if (typeof (home) == 'undefined') {
  var home = {
    display: function (abtest) {
    }
  }
};

https://developer.mozilla.org/en-US/docs/Web/API/Window.home

So I guess this is the same issue as Bug 989584.

Comment 2

[Kohei Yoshino [:kohei]]

Specifically this causes the error:

$(document).ready(function() {
  home.display('A');
});

Comment 3

[Kohei Yoshino [:kohei]]

Attachment: testcase

Comment 4

[Boris Zbarsky [:bzbarsky]]

Hrm.  If other browsers don't support window.home(), can we just remove it?  I doubt anyone would seriously use this anyway; I see no uses in our tree or in addons.

Comment 5

[Masatoshi Kimura [:emk]]

Neither IE11 nor Chrome has this function.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Thanks Kohei for checking that. It clearly saves me the time for a regression test.

Comment 7

[Hallvord R. M. Steen [:hallvors]]

Sure, let's kill window.home()

Comment 8

[:Gijs (he/him)]

(In reply to Boris Zbarsky [:bz] from comment #4)
> Hrm.  If other browsers don't support window.home(), can we just remove it? 
> I doubt anyone would seriously use this anyway; I see no uses in our tree or
> in addons.

Actually, I see a little usage in e.g. https://addons.mozilla.org/en-US/firefox/addon/trafficlight/ and https://addons.mozilla.org/en-US/firefox/addon/web-advisor/ . (which is completely orthogonally interesting because these differently branded add-ons seem to share a lot of code...) . I'd still argue we should probably just remove this though.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Yeah, looks like addons mxr was lying to me.  I see some uses in there.

I'll just make this chromeonly.  Peter was OK with nuking this from the web.  I'll probably do the same with back() and forward() while I'm here.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Hide window.home/back/forward from web pages, since those are non-standard and cause pages to not set up their own thins with the same names

Comment 11

[Boris Zbarsky [:bzbarsky]]

For Firefox 30 we'll handle this via bug 976920.

For Firefox 31 and later I'd like to backport this patch.

Also, the code cited in comment 1 doesn't do (luckily) what its author thinks it does...

Comment 12

[Sylvestre Ledru [:Sylvestre]]

I have a friend working at Deezer. Let me know if I can help.

Comment 13

[Boris Zbarsky [:bzbarsky]]

We don't need them to do anything, thanks!

Comment 14

[Hallvord R. M. Steen [:hallvors]]

Boris: for the record: I'm more worried about nuking back() and forward() than home(). Please consider removing home() only. 

Just gut feeling, but I think there is significant usage of back(), much less usage of forward() but likely some. If we have some telemetry / usage measurements you could enable for these methods it might tell us more about their "removability"..

Comment 15

[Kohei Yoshino [:kohei]]

I was thinking the same thing, but seems like window.back() and window.forward() are Gecko-specific obscure methods. People are using the standard history.back() and history.forward() methods implemented in all browsers. It might be safe to remove these.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Right.  history.back()/forward() are used a lot.  I would expect that window.back()/forward() are not.

I did consider removing only home(), but I would expect that the compat worries from name collisions on back/forward are bigger than the ones from the removal...

I could add telemetry (though we don't have a sane system for doing it for this sort of thing), but I'm not sure we need it in this case.

Comment 17

[Hallvord R. M. Steen [:hallvors]]

https://github.com/search?l=javascript&p=1&q=%22window.back%28%29%22&ref=cmdform&type=Code - unfortunately I haven't found a way to use a real literal string (or regexp) search on github..

Comment 18

[Boris Zbarsky [:bzbarsky]]

Hmm.... http://jsfiddle.net/geZ8H/ says only Presto and Gecko support this.

I'm willing to take the chance on those functions.

Comment 19

[Kohei Yoshino [:kohei]]

I can see some on GitHub but most of them are probably Firefox extensions or Firefox OS apps. [ChromeOnly] won't affect extensions, and the latter case is subject to tech evang. We can't help legacy, unmaintained "Best viewed with Netscape" sites but those might have already been broken for other reasons.

Comment 20

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/55d1924160c9

Comment 21

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8425551 [details] [diff] [review]
Hide window.home/back/forward from web pages, since those are non-standard and cause pages to not set up their own thins with the same names

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 932322 
User impact if declined: Some sites will not work correctly, unless we backport
   bug 976920 to Gecko 31.
Testing completed (on m-c, etc.): Passes tests
Risk to taking this patch (and alternatives if risky): Risk is medium to low.
    The only other UA that implemented these functions was Presto, and there are
    equivalents for back/forward on window.history that work for all objects. 
    Extensions should continue to work OK.
String or IDL/UUID changes made by this patch: None.

Comment 22

[Kohei Yoshino [:kohei]]

https://developer.mozilla.org/en-US/docs/Web/API/Window
https://developer.mozilla.org/en-US/docs/Web/API/Window.back
https://developer.mozilla.org/en-US/docs/Web/API/Window.forward
https://developer.mozilla.org/en-US/docs/Web/API/Window.home
https://developer.mozilla.org/en-US/Firefox/Releases/31/Site_Compatibility

Comment 23

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/55d1924160c9

Comment 24

[Ryan VanderMeulen [:RyanVM]]

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

Comment 25

[Lukas Blakk [:lsblakk] use ?needinfo]

Bug 932322 is disabled on 30 so is this really affected there?

Comment 26

[Boris Zbarsky [:bzbarsky]]

It's not now (but was when the bug was filed).

Comment 27

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Works great with Mozilla/5.0 (X11; Linux x86_64; rv:32.0) Gecko/20100101 Firefox/32.0 ID:20140522030204 CSet: b40296602083

Thanks for the quick fix!

Comment 28

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Same with Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Firefox/31.0 ID:20140522004003 CSet: 5239876497e2