Closed Bug 1162249 Opened 9 years ago Closed 9 years ago

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox39 --- unaffected
firefox40 + fixed

People

(Reporter: ntim, Unassigned)

References

Details

(Keywords: regression)

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
Well, this is exciting.  hg bisect says:

The first bad revision is:
changeset:   239491:dfdcce6b319a
user:        Jeff Walden <jwalden@mit.edu>
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
Blocks: 748550
Component: General → JavaScript Engine
Flags: needinfo?(jwalden+bmo)
[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.
Tracked for 40.
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.
Flags: needinfo?(jwalden+bmo)
...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.
(...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.)
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?
Flags: needinfo?(jwalden+bmo)
(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.
Flags: needinfo?(jwalden+bmo)
Telemetry is probably a good idea, yes, as it is for every backwards-incompatible change.
I have contacted the website admin about this. I will inform you once they answer.
Woo, thanks a ton!
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.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WORKSFORME
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.
Based on the fact that the bug fix for 1164741 has been uplifted to Aurora, setting status-firefox40 to "Fixed".
You need to log in before you can comment on or make changes to this bug.