897784 - Addition of Array.prototype.find breaks immobilienscout24.at

Status: RESOLVED WORKSFORME

(Tech Evangelism Graveyard :: German, defect)

Description

[Sören Hentzschel]

Attachment: bad

STR:

1. go to www.immobilienscout24.at
2. click on the input field "Bundesland, Ort oder Postleitzahl"
3. select a city

Expected result:

Page shows results.

Actual result:

Page doesn't show results. See screenshots.

First bad revision (mozregression):

Die erste fehlerhafte Revision ist:
Änderung:        136603:a839287871f9
Nutzer:          Till Schneidereit <[e-mail]>
Datum:           Wed Jun 26 18:47:41 2013 +0200
Zusammenfassung: Bug 885553 - Implement ES6 Array.prototype.find and Array.prototype.findIndex. r=jwalden

Comment 1

[Sören Hentzschel]

Attachment: good

Comment 2

[Till Schneidereit [:till]]

Yeah, this is caused by the site adding a `find` method to arrays that's incompatible with the es6 version. I whish they did that unconditionally, instead of only if no method of the same name is defined.

Moving to Tech Evangelism for now, but CC-ing people in preparation of maybe taking measures on our side.

Comment 3

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

contacted on https://immobilienscout24.secure.force.com/arlFAQKontakt

Comment 4

[bhavana bajaj [:bajaj]]

Do we know of any other major websites that may have been impacted already ?

Comment 5

[Till Schneidereit [:till]]

(In reply to bhavana bajaj [:bajaj] from comment #4)
> Do we know of any other major websites that may have been impacted already ?

I'm not aware of any, no. IMO, we should let this ride the trains for now, but be prepared to back it out if we encounter problems.

The argument against that would be that platform or addons code might start relying on it, making it harder to back out.

Comment 6

[Kohei Yoshino]

The cause could be the Sugar library which extends Array.prototype:
https://github.com/andrewplummer/Sugar/blob/master/lib/array.js#L405

The find and findIndex methods are affected by the native ES6 methods.

I'll add this issue to the site compatibility doc:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/25/Site_Compatibility

Comment 7

[Kohei Yoshino]

Here's the API doc: http://sugarjs.com/api/Array/find

Comment 8

[Kohei Yoshino]

Note that Suger also has http://sugarjs.com/api/Array/from, which will be broken when Bug 866849 is landed. Furthermore, http://sugarjs.com/api/String/repeat might be broken since Firefox 24 as the native method has been added by Bug 815431.

Comment 9

[Till Schneidereit [:till]]

There's a new version of SugarJS, so ideally the site would upgrade to that. In case upgrading to a whole new version proves too time-consuming, there's also a patch for just the problematic builtins. The details can be found in bug 903755 comment 10.

Comment 10

[Alex Keybl [:akeybl]]

(In reply to Hallvord R. M. Steen from comment #3)
> contacted on https://immobilienscout24.secure.force.com/arlFAQKontakt

Any updates?

Comment 11

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

No news. Tried contacting them again - now also posting on their Facebook and Google+ pages.

Comment 12

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

(BTW - I guess this shouldn't be a blocker - AFAIK we don't usually consider tech evangelism issues blockers..)

Comment 13

[Maher Chamsi]

we fixed the bug by patching sugarJS (see https://github.com/andrewplummer/Sugar/blob/master/lib/patches/sugar-es6-patch.js). 
fyi the page you've tested has been replaced by a new one.

please let me know if the bug is still occurring.

Comment 14

[Till Schneidereit [:till]]

Maher, thanks for the confirmation. Given that you replaced the problematic page, I'm going to mark this as resolved without any more testing. I have confirmed the SugarJS patch to fix the underlying issue already, so if that is applied, we should be good here.