Last Comment Bug 789036 - MooTools 1.2.x was broken by adding String.prototype.contains
: MooTools 1.2.x was broken by adding String.prototype.contains
Status: RESOLVED FIXED
fixed by MooTools 1.2.6 (and higher)
: site-compat
Product: Tech Evangelism Graveyard
Classification: Graveyard
Component: English US (show other bugs)
: Trunk
: All All
: -- normal
: ---
Assigned To: english-us
:
Mentors:
: 834306 (view as bug list)
Depends on: 781796 784280 786584 788483 789093 790462 791980 801311 805389 812578 821175 828326 829118 829384 830130 830189 830779 831388 835051 836667 858943
Blocks: harmony:stringextras 793781
  Show dependency treegraph
 
Reported: 2012-09-06 06:12 PDT by :Benjamin Peterson
Modified: 2015-04-19 23:39 PDT (History)
18 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments

Description :Benjamin Peterson 2012-09-06 06:12:18 PDT
This is a meta bug for breakage caused by bug 772733's addition of String.prototype.contains. MooTools defines a incompatible version of String.prototype.contains. Versions of MooTools greater than 1.2 override the builtin implementation, but 1.2.x assumes it is a compatible implementation. Upgrading to any later version of MooTools fixes the problem.

A MooTools bug has been filed:
https://github.com/mootools/mootools-core/issues/2402
Comment 1 Gian-Carlo Pascutto [:gcp] 2012-09-06 06:31:15 PDT
Do we know how widespread this breakage is? MooTools claims to have somewhere between 5-8% marketshare, it's probably realistic that quite some of those aren't updating their framework regularly, if at all.

This might end up breaking a reasonable (>1%) amount of sites...
Comment 2 :Benjamin Peterson 2012-09-06 06:36:58 PDT
For historical information about a similar snafu with Function.prototype.bind, see bug 784280 comment #10.
Comment 3 Boris Zbarsky [:bz] (TPAC) 2012-09-18 13:32:13 PDT
I think we should strongly consider disabling this on beta to avoid the breakage for the moment while continuing to evangelize the sites involved....
Comment 4 :Benjamin Peterson 2012-09-18 13:34:50 PDT
It's not in beta yet, but yes I agree. We can wait at least another month, though.
Comment 5 Alex Keybl [:akeybl] 2012-09-21 17:04:52 PDT
(In reply to Benjamin Peterson [:benjamin] from comment #4)
> It's not in beta yet, but yes I agree. We can wait at least another month,
> though.

I don't want our users to have a bad beta experience (even for a couple of betas), unless we think it's the only leverage for sites to fix. Were you thinking about fixing it for FF16b1 or later?
Comment 6 :Benjamin Peterson 2012-09-22 17:18:32 PDT
(In reply to Alex Keybl [:akeybl] from comment #5)
> (In reply to Benjamin Peterson [:benjamin] from comment #4)
> > It's not in beta yet, but yes I agree. We can wait at least another month,
> > though.
> 
> I don't want our users to have a bad beta experience (even for a couple of
> betas), unless we think it's the only leverage for sites to fix. Were you
> thinking about fixing it for FF16b1 or later?

I propose beta 2 or 3.
Comment 7 Boris Zbarsky [:bz] (TPAC) 2012-09-22 17:23:46 PDT
Our "beta"s are actually release candidates.  That's how we're pitching them, and that's how we should treat them.

We shouldn't be shipping known bugs on beta if we can avoid it, imo.  That's what Aurora is for...
Comment 8 :Benjamin Peterson 2012-09-22 17:32:35 PDT
I should note browser JS has started depending on contains.
Comment 9 Boris Zbarsky [:bz] (TPAC) 2012-09-22 17:43:04 PDT
That's even more of a reason to not ship it in beta.

In fact, if extensions are going to depend on it, then we can't ship it in beta, and ideally shouldn't be shipping it in Aurora, because we have extension compat promises we make with Aurora and beta builds...
Comment 10 Jeff Walden [:Waldo] (remove +bmo to email) 2012-09-22 22:41:17 PDT
(In reply to Boris Zbarsky (:bz) from comment #9)
> In fact, if extensions are going to depend on it, then we can't ship it in
> beta, and ideally shouldn't be shipping it in Aurora, because we have
> extension compat promises we make with Aurora and beta builds...

Extensions don't have to depend on it unless they specifically opt in.  Seeing as we didn't tell extension authors about any of the new string methods (going by bug 772733 still having dev-doc-needed, and there not being a page for String.prototype.contains yet, and if I say anything more right now I'm going to sound disturbingly like philor), and seeing as if they're targeting nightly or Aurora I think we have to presume they know they're playing with fire, I don't see that we need to disable this in Aurora too.
Comment 11 Boris Zbarsky [:bz] (TPAC) 2012-09-23 06:26:53 PDT
I can sort of buy that.  For beta, though, we promise extensions basically no breaking change of any sort after beta1, unless it's fixing a stop-ship bug kinda thing.  So we absolutely need to disable the string methods before going to beta.
Comment 12 Alex Keybl [:akeybl] 2012-09-24 11:06:09 PDT
(In reply to Boris Zbarsky (:bz) from comment #9)
> That's even more of a reason to not ship it in beta.
> 
> In fact, if extensions are going to depend on it, then we can't ship it in
> beta, and ideally shouldn't be shipping it in Aurora, because we have
> extension compat promises we make with Aurora and beta builds...

I agree with this. Nobody's made the case for how trying to leverage beta to get a MooTools fix outweighs the add-on/browser risk to disabling later in the cycle. I'll clone for the in-product change and leave this open for TE.
Comment 13 :Benjamin Peterson 2012-10-06 09:17:15 PDT
Fixed in bug 793781 for 17, so untracking there.
Comment 15 Alex Keybl [:akeybl] 2012-11-12 15:58:32 PST
Untracking in favor of bug 793781.
Comment 16 :Benjamin Peterson 2012-12-01 08:16:44 PST
MooTools 1.5 will have a ES6 compatible contains signature!

https://github.com/arian/mootools-core/commit/f9307e0c9dcae6a63b8f905c0b977368179d3d8b
Comment 17 Daniel Buchner [:dbuc] 2012-12-04 15:49:17 PST
Hey guys, I used to slang code for MooTools and I can run point on contacting and coordinating with them to address this. What is the honeymoon scenario we're looking for here?
Comment 18 :Benjamin Peterson 2012-12-05 16:53:46 PST
Ideally, all the dependent bugs would be closed. :) I've tried to contact them all, but feel free send your own.
Comment 19 Boris Zbarsky [:bz] (TPAC) 2013-01-10 06:16:06 PST
Looks like we shipped this breakage in Firefox 18.  Just for post-mortem purposes, was that a conscious decision, or did it just slip through the cracks?
Comment 20 Boris Zbarsky [:bz] (TPAC) 2013-01-10 06:40:04 PST
Documented at <https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_18#Mootools_1.2.x_is_not_compatible_with_Firefox_18_and_newer>.  If we _did_ ship this on purpose, I'd like to understand why we didn't bother documenting it or relnoting it or anything.
Comment 21 :Benjamin Peterson 2013-01-10 07:27:00 PST
It was a conscious decision. At least in the case of HP, they told me several times they were going to fix it by now. Trust but verify, I suppose.
Comment 22 Boris Zbarsky [:bz] (TPAC) 2013-01-10 07:28:45 PST
OK.  Next time we ship something like this on purpose, please make sure it's relnoted and documented in our site compat docs, ok?
Comment 23 Brendan Eich [:brendan] 2013-01-11 18:08:00 PST
(In reply to :Benjamin Peterson from comment #21)
> It was a conscious decision. At least in the case of HP, they told me
> several times they were going to fix it by now. Trust but verify, I suppose.

HP is just one instance of the problem. Why should Firefox risk losing market share to take this hit? We need to push back in Ecma TC39 and get other browsers making the same change *after* a coordinated evangelism effort.

Should this get disabled in 18.0.1? It seems so from all the comments here.

/be
Comment 24 :Benjamin Peterson 2013-01-12 20:21:27 PST
(In reply to Brendan Eich [:brendan] from comment #23)
> (In reply to :Benjamin Peterson from comment #21)
> > It was a conscious decision. At least in the case of HP, they told me
> > several times they were going to fix it by now. Trust but verify, I suppose.
> 
> HP is just one instance of the problem. Why should Firefox risk losing
> market share to take this hit? We need to push back in Ecma TC39 and get
> other browsers making the same change *after* a coordinated evangelism
> effort.

Push back into ECMA TC39 for what?

> 
> Should this get disabled in 18.0.1? It seems so from all the comments here.

I don't think I'm one to make such a decision. I should not it's not completely trival because chrome code is using contains().
Comment 25 Matthias Versen [:Matti] 2013-01-24 20:18:13 PST
*** Bug 834306 has been marked as a duplicate of this bug. ***
Comment 26 Swarnava Sengupta (:Swarnava) 2013-01-26 20:21:08 PST
*** Bug 835051 has been marked as a duplicate of this bug. ***
Comment 27 David Weir (satdav) 2013-01-27 04:59:05 PST
I am from support and don't see a lot of users complaining about this issue
Comment 28 Jeremy Lwanga 2013-01-27 08:07:37 PST
(In reply to David Weir (satdav) from comment #27)
That's probably because a lot of the users haven't upgraded to version 1.8 and haven't visited a site using mootools 1.2xx

You can see an issue here; try replying to this - http://www.techrepublic.com/forum/discussions/103-401241, firefox is failing to getParent(<selector>)

Here's another example, the social toolbar here [http://www.techrepublic.com/blog/window-on-windows/the-first-10-things-you-should-do-to-a-new-windows-8-desktop-installation/6467] should have a more button at the end. Check it out in other browsers or older versions of firefox
Comment 29 Swarnava Sengupta (:Swarnava) 2013-01-27 08:46:23 PST
Go to http://foto-treff-bielefeld.de/
try to visit any link from black navigation bar
if you click, nothing happening
Comment 30 Matthias Versen [:Matti] 2013-01-29 09:01:43 PST
*** Bug 834306 has been marked as a duplicate of this bug. ***
Comment 31 Daniel Buchner [:dbuc] 2013-02-12 08:06:41 PST
(In reply to Brendan Eich [:brendan] from comment #23)
> (In reply to :Benjamin Peterson from comment #21)
> > It was a conscious decision. At least in the case of HP, they told me
> > several times they were going to fix it by now. Trust but verify, I suppose.
> 
> HP is just one instance of the problem. Why should Firefox risk losing
> market share to take this hit? We need to push back in Ecma TC39 and get
> other browsers making the same change *after* a coordinated evangelism
> effort.
> 
> Should this get disabled in 18.0.1? It seems so from all the comments here.
> 
> /be

As this affects quite old version of the MooTools library, specifically the 1.2.x branch which is now ~5 years old, MooTools will be releasing a 1.2.6 for that legacy code which corrects the issue for MooTools users. the 1.3+ branches of the library are all fine and have had no issues.

We'll need to updated our engagement actions on this front and push people to the new version. There's a blog post imminent that will describe all of this and includes the new code, I'll link to it here as soon as it's released.
Comment 32 Loic 2013-03-04 05:39:05 PST
MooTools 1.2.6 Released: http://mootools.net/blog/2013/02/19/mootools-1-2-6-released/
This is a new maintenance release for the old 1.2 series. The new ECMAScript 6 specification has a proposal for String.prototype.contains that unfortunately conflicts with the MooTools implementation of contains.
Firefox 18 already ships this new version of contains. This is not a problem for MooTools 1.3.x and onward, but this breaks MooTools 1.2.5, especially the code inside the MooTools framework that utilized this method, like selectors.
Comment 33 Hallvord R. M. Steen [:hallvors] 2013-07-19 13:19:03 PDT
This can be resolved since MooTools shipped the fix already..

Note You need to log in before you can comment on or make changes to this bug.