1162249 - https://0920145h.index-education.net/pronote/parent.html?login=true doesn't work in Firefox Nightly

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect)

Description

[Tim Nguyen :ntim]

STR :
- Go to https://0920145h.index-education.net/pronote/parent.html?login=true

AR :
- You will see "Erreur sur le chargement de la page. Veuillez vider le cache de votre navigateur." as alert
It asks you to clear the cache in order to get the site to work. But clearing the cache doesn't fix things.

ER :
- The site should load properly

--
Reproducible on :
- Firefox OS Nightly
- Firefox Nightly 40
- Blank profile

Comment 1

[Boris Zbarsky [:bzbarsky]]

I _think_ the regression range here is http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a35163f83d22&tochange=51e3cb11a258

Comment 2

[Boris Zbarsky [:bzbarsky]]

Well, this is exciting.  hg bisect says:

The first bad revision is:
changeset:   239491:dfdcce6b319a
user:        Jeff Walden <[e-mail]>
date:        Thu Apr 02 21:08:11 2015 -0400
summary:     Bug 748550 - Remove support for |for (... = ... in/of ...)| now that ES6 has removed it.  r=jorendorff

Comment 3

[Boris Zbarsky [:bzbarsky]]

[Tracking Requested - why for this release]:

OK, so https://0920145h.index-education.net/pronote/E_2_C_3A01F07896686B4E016DC7781D12BC731C4E6D6448155992AD10027AC089EFC7/parent.js has this bit in it, around line 1805:

   for(var x=0 in lResponses)

where afaict lResponses is an array.

I just checked and this script:

    for (var x = 0 in [2, 3]) document.write(x);

works in shipping Firefox, Safari, IE, and Chrome, and writes out "01".  So it looks to me like making this not work (if that was the intent) may well not be web-compatible.

Comment 4

[jen strickland :jenstrickland]

Tracked for 40.

Comment 5

[Jeff Walden [:Waldo]]

I might be able to hack back in something to temporarily and/or not quite so remove this support, by just ignoring the assignment altogether.  But I refuse to accept that *two* sites breaking is incontrovertible evidence that this is not web-compatible.  That's  letting the terrorists win.  I'll send mail to them and see whether we can get them to fix this.  That other browsers implement something here *is not* evidence that this can't be changed.

Note that removing this support *is* basically necessary to avoid rather severe semantic problems with respect to the block scoping changes to for-loops introduced in ES6.  There really is no meaningful way to support some new ES6 semantics *and* this old garbage.

Comment 6

[Jeff Walden [:Waldo]]

...well, maybe I *directly* can't send them mail, because my French is at the Bugs Bunny level.  Somehow I'll figure out something to say and deputize someone to help with the language thing.

Comment 7

[Jeff Walden [:Waldo]]

(...the "letting the terrorists win" thing is obvious hyperbole/joke/not to be taken literally at all, to be clear, just in case any website authors manage to stumble across this and think that's serious in any way at all.  Sigh, humor and prickliness these days.)

Comment 8

[Boris Zbarsky [:bzbarsky]]

There is a difference between "two sites breaking" and "two breaking sites reported within three weeks of the change landing on m-c".  The real question is how many have gone unreported or unfound so far, but will cause problems for users on release builds.

Given that every single UA supports this syntax right now, I'm not too sanguine about it just being the two sites...

At the very least, have we considered coordinating shipping this change with other UAs, so we're not doing the whole "hey, it won't work in our browser but it'll work in all the others" thing?

Comment 9

[Jeff Walden [:Waldo]]

(In reply to Not doing reviews right now from comment #8)
> There is a difference between "two sites breaking" and "two breaking sites
> reported within three weeks of the change landing on m-c".  The real
> question is how many have gone unreported or unfound so far, but will cause
> problems for users on release builds.

So, bluntly, the usual FUD and handwavery without rigorous extrapolation from statistically significant data.

> At the very least, have we considered coordinating shipping this change with
> other UAs, so we're not doing the whole "hey, it won't work in our browser
> but it'll work in all the others" thing?

I'm considering it now.  But obviously it's better not to do so and waste umpteen people's time if it can be done without any coordination at all.  Maybe it's time to go harder now, or try telemetry preliminarily.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Telemetry is probably a good idea, yes, as it is for every backwards-incompatible change.

Comment 11

[Tim Nguyen :ntim]

I have contacted the website admin about this. I will inform you once they answer.

Comment 12

[Jeff Walden [:Waldo]]

Woo, thanks a ton!

Comment 13

[Tim Nguyen :ntim]

Here's an official answer from them :
> 1 – à ce jour, il s’agit d’une version « beta » de Firefox.
That basically means that since it's a beta version, they probably won't fix it until 40 reaches release.

2 – Notre service technique est au courant et gère cela pour les versions 2015.
That means it is a known issue to them, and they will do any necessary change on their side for the next versions.

3 – aucune certitude à ce jour sur la pérennité de cette modification. On surveille et on fera le nécessaire dans les versions 2015.
That means the change is still unsure on Firefox's side and they'll do anything necessary on their side in the next versions.

Comment 14

[Tim Nguyen :ntim]

Here's an official answer from them :
> 1 – à ce jour, il s’agit d’une version « beta » de Firefox.
That basically means that since it's a beta version, they probably won't fix it until 40 reaches release.

> 2 – Notre service technique est au courant et gère cela pour les versions 2015.
That means it is a known issue to them, and they will do any necessary change on their side for the next versions.

> 3 – aucune certitude à ce jour sur la pérennité de cette modification. On surveille et on fera le nécessaire dans les versions 2015.
That means the change is still unsure on Firefox's side and they'll do anything necessary on their side in the next versions.

Comment 15

[Jeff Walden [:Waldo]]

Bug 1164741 added back *parsing* support for this bogus syntax, by ignoring the initializer and ascribing it no-op semantics.  That gets this site back to working again...for now.  In the longer run I think we still should remove this syntax, so the site here *should* still update its code.  But there's no need to rush it -- just make the change and let the site pick it up whenever the next update happens.

Comment 16

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

Based on the fact that the bug fix for 1164741 has been uplifted to Aurora, setting status-firefox40 to "Fixed".