jQuery 1.8+ Breaks Core JavaScript

Yes, I've noticed this too.

Patches welcome :)

This i a bug in jQuery (http://bugs.jquery.com/ticket/12159)
Line #876 in "modules/overlay/overlay-parent.js" returns an object.

JQ1.8
lastDisplaced.offset().top+lastDisplaced.outerHeight()
= "0[object Object]"

JQ1.7
lastDisplaced.offset().top+lastDisplaced.outerHeight()
= 65

//are monkey patches ok?

if(jQuery && jQuery.fn.jquery == "1.8.2") {
jQuery.fn._outerHeight = jQuery.fn.outerHeight;
jQuery.fn.outerHeight = function() { return jQuery(this)._outerHeight(false); }
}

Made a patch...

Since jQuery Update is the project creating the problem (by allowing jQuery 1.8+), shouldn't it solve the problem as well (instead of patching Drupal core JavaScript files)?

The bug is in jQuery's .outerWidth and .outerHeight functions in 1.8+, which Drupal Core uses in a few places. Any solution needs to involve fixing these functions, not rewriting core to avoid using them.

The bug in jQuery is that .outerWidth([includeMargin]) and .outerHeight([includeMargin]) (overlay-parent.js uses both of these!) are both broken in jQuery 1.8+, and return a jQuery object instead of a number when called without the includeMargin parameter.

This is a bug because the jQuery docs say that either no parameter or false should work the same. The easy, universal fix is to override outerWidth and outerHeight to default to includeMargin = false.

Here's an example fix. All it does is call the real jQuery functions with false instead of undefined when the includeMargin parameter is missing.

(function() {
  var original_outerWidth = jQuery.prototype.outerWidth;
  var original_outerHeight = jQuery.prototype.outerHeight;
  jQuery.prototype.outerWidth = function(includeMargin) {
    return original_outerWidth.call(this, includeMargin || false);
  };
  jQuery.prototype.outerHeight = function(includeMargin) {
    return original_outerHeight.call(this, includeMargin || false);
  };
})();

I've hand-minified a version of that function, which I'm using in my own template's .js to temporarily solve the problem:

(function(p){var o='outer',w='Width',h='Height',a=function(f){var f=f;return function(i){return f.call(this,i||false);};};p[o+w]=a(p[o+w]);p[o+h]=a(p[o+h]);})(jQuery.prototype);

the bug in jquery has been marked invalid BTW

http://bugs.jquery.com/ticket/12293

Ticket http://bugs.jquery.com/ticket/12293 is just one of literally dozens of tickets covering this issue, so it's hard to say what the true status is. Regardless, it's undeniably a bug:

1. The spec on outerWidth says
   

   If includeMargin is omitted (ed: emphasis is mine) or false, the padding and border are included in the calculation; if true, the margin is also included.

2. Both the spec on outerWidth and outerHeight indicate that the includeMargin parameter is optional.
3. This behavior used to work as the spec describes before jQuery 1.8. There are many, many examples of code (in Drupal and other projects) using .outerWidth() and .outerHeight() without any parameter.
4. Both specs say the return value of the call is a number or null, (never an object):
   

   Returns the width of the element, along with left and right padding, border, and optionally margin, in pixels.

   Get the current computed height for the first element ... Returns an integer (without "px") representation of the value or null if called on an empty set of elements.

   After 1.8, it sometimes returns an object.

It used to work, the specs say it should work, but it no longer works. That's pretty clearly a bug. My proposed fix is simple and should be non-disruptive: all we do is supply false if the optional includeMargin parameter is missing. Then jQuery 1.8+ is happy, and all existing code will continue to work as the jQuery docs say it should.

Any solution needs to involve fixing these functions, not rewriting core to avoid using them.

It's totally fair, it's JS anything goes. We sure like bikesheading in core but we don't bite #2018791-6: states.js is not compatible with jquery +1.6.1 because it use $.attr in the wrong way.

Both specs say the return value of the call is a number or null, (never an object):

< troll>null is an object< /troll>

More generally, feel free to open core bugs against D7 if the change isn't too drastic and it makes it easier on jquery_update.

< troll>null is an object< /troll>

Let me be more specific:
Not a jQuery object, which is what it returns in jQuery 1.8+.

I would open a core bug, except that the changes in jQuery 1.8 will likely impact contrib as well, so the solution seems most fitting to apply universally: where jQuery 1.8 is used (which only happens with jQuery Update), we solve the problem it introduces.

My rationale is based on two possible solutions:

1. We can update core JS to always pass false as the optional (except effectively it's no longer optional in jQuery 1.8+, even though the docs still say it is) 2nd parameter to outerWidth() and outerHeight(), and hope that fixes everything. This is a bad idea, because who knows what other contrib modules may use those functions, based on any version of jQuery prior to 1.8. That means running jQuery Update + contrib will open up possible errors down the road. We would then have to file issues and fix every one of those otherwise-valid Drupal 7.x contrib modules, just because jQuery 1.8+ has a bug and jQuery Update is enabling jQuery 1.8+. This seems the wrong way. Fixing it in core will not fix contrib.
2. Or, we can simply patch jQuery Update so that when jQuery 1.8+ is enabled, it doesn't change the outerWidth() and outerHeight() functionality, and all existing contrib and core JS will run without any problem. This is totally backwards, forwards, all directions compatible, non-side-effecting, safe and easy. This seems the right way. The very tiny JS in #5 does this.

In that case sure, fixing core doesn't sound like the right solution. But in case people had the wrong idea, it's not impossible to patch core for the right issue.

Lots of people here I haven't seen in the core queue. Just doing some light poking/recruiting, don't mind me :D

We probably all think Drupal Core is scary :)

Here's a patch that integrates the JS from #5 and automatically inserts it if the requested jQuery version is 1.8+.

I noticed when rolling the patch against the latest 7.x-2.x-dev that there's a new replace/misc/1.9/overlay-parent.js that jQuery Update is overriding core with, but it looks like (from overlay-parent.diff.js) it isn't doing anything to solve the outerWidth/outerHeight problem, so I don't think it conflicts with this fix.

This isn't needed with -dev as far as I can tell, which fixes the incomportable versions of jquery/jquery ui http://bugs.jquery.com/ticket/12491#comment:2

That ticket seems to be about jQuery UI solving the problem (by changing how they use the outerWidth/outerHeight functions), not the jQuery bug getting fixed. So, the problem still remains for core/contrib scripts written against any version of jQuery < 1.8 (unless I'm reading something incorrectly, which is always a possibility!).

Using jquery update stable, $().outerWidth returned an object; using -dev outerWidth returned the width

This sounds like a jQuery version/API deprecation issue and can resolved in the 7.x-3.x branch by either:

1. Choosing a lower jQuery version for an admin theme like Seven
2. Enabling the jQuery Migrate plugin to handle deprecated APIs/features.