From jQuery 1.6 To access a property you should use prop instead of attr. From 1.6.1 attr still accept property again (http://blog.jquery.com/2011/05/10/jquery-1-6-1-rc-1-released/) for backward compatibility but behave a little differently (in witch way exactly?). This breaking a lot of modules's javascript.

Possibilities

Extend jquery so it still supports attr

Extend jquery so attr behave the same old way (like 1.5). We can use hook_attr or just overwrite the all attr function. Of course we can keep the original attr on an other name (attr_org) in case we want to use it.

Rewrite script case by case

We can overwrite every scripts we find than had a bug. The problem after is than we have to maintain that!

//This check determines if using a jQuery version 1.7 or newer which requires the use of the prop function instead of the attr function when not called on an attribute. If we want it's works for 1.7 and 1.4 we have to check if prop is available with something like that:
if(typeof $().prop == 'function') {
  object.prop('disabled', true);
  object.children().prop('disabled', true);
}
else {
  object.attr('disabled', true);
  object.children().attr('disabled', true);
}

Related issues

#1494860: Views Rewrite Results UI Broken using JQuery 1.7
#1495386: jQuery 1.7 breaks CTools dependent.js
#1448494: jQuery 1.7: field_ui.js use attr for property
#1448490: Remove the states.js overwrite as it was fixed upstream
#2018791: states.js is not compatible with jquery +1.6.1 because it use $.attr in the wrong way

Comments

damontgomery’s picture

I think the problem with this will be that we will need an exhaustive list of when to use attr and when to use prop, or some other method for checking.

http://api.jquery.com/prop/ Suggests that the prop should be used mostly for inputs and checkboxes, in essence form items. This may limit the list quite a bit.

In addition to this, how are you going to take into account all of the different ways conditions can be passed to the prop function when checking for this?

.prop(propertyName)
.prop(propertyName, value)
.prop(map)
.prop(propertyName, function(index, oldPropertyValue))

It's definitely possible, but it seems like something that will need to be worked on quite a bit.

I don't know if it's the better option, but I think that patching a few of the most popular modules is a quick and easy method to solve this issue. My patch, which you copied above, does not prevent the use of the jquery extension, and fixes the problem until the "better" solution is reached.

My main thrust here is that everything with Drupal is on a volunteer process and I don't know when or who will be able to work on this proposed solution. In the mean time, we would like to switch to jquery 1.7 and continue to use the popular modules such as ctools and views. I am capable of the second solution and willing to volunteer some of my time to work towards that solution, as has already been done.

Another thing to consider is that at one point, jquery earlier than 1.6 will stop working in some capacity on Chrome. At that point, I expect a flood of demand for jquery 1.7 and consequently one of the two solutions presented. Someone will either extend jquery for backwards compatibility, or once everyone is forced to use 1.7 and thus break all their modules, patches will be issued to fix the individual cases.

TLDR: I think it's great to work on a more comprehensive fix, but I think small patches will fix 99% of users' problems in the mean time.

gagarine’s picture

You should use prop for property when you want to change a property value (and not their initial value):

autofocus, autoplay, async, checked, controls, defer, disabled,
hidden, loop, multiple, open, readonly, required, scoped, selected

I think making a backward compatible function should take 20 lines on JS... Changing on all modules is going to take a lot more patch. My main concern is keeping all the JS we replace up to date.

Some experiments http://jsfiddle.net/gagarine/S2qGg/

gagarine’s picture

Playing with jsfiddle finally make me understood what append.

Changing the property has not a different behavior.

Those will in have the same behavior in all version (1.5 to 1.7)

foo.attr("checked", false); //set to false
foo.attr("checked", ''); //set to false
foo.attr("checked", "checked"); //set to true
foo.attr("checked", true); //set to true

But reading return different value depending of the version

foo.attr("checked"); //read the value

In 1.5 it will return true or false depending if it's checked or not.
In 1.6.1+ it will return undefined or "checked".

So this is it. This is the things the break your code...

Example of "bad" code than will break with 1.6 and 1.7:

var val = $(trigger).attr('checked') || 0;

Hop I don't miss something.

EDIT:

from the issue on http://bugs.jquery.com/ticket/9856

The suggested way to check if a checkbox is checked has always been to check for a truthy value. For instance:

if ( $('input:checkbox').attr('checked') ) {}

instead of

if ( $('input:checkbox').attr('checked') === true ) {}

Any code that was written in the recommended way would not have to be changed.

So first we should fix inside contrib projects and drupal core by using correctly attr but without testing witch version we are using. After can eventually make a small wrapper if we have the need.

damontgomery’s picture

Ok,

So in #1494860: Views Rewrite Results UI Broken using JQuery 1.7, the only thing that needed to change was the first part. I'll look into this when I have the chance and request a change as you described. There should be no reason against making this patch if it works for all jquery versions, but also fixes errors with jQuery 1.7.

Thanks for the exploration and description / documentation of the problem and solution.

gagarine’s picture

FYI I proposed two changes in drupal core so jquery 1.7 will works #1480568: use $.attr with false instead of empty string without the need of #1448494: jQuery 1.7: field_ui.js use attr for property

gagarine’s picture

We should add #230771: Provide basic javascript syntax and comment review in coder so it will be very easy to correct that in modules and core...

We can after certainly remove some files we overwrite.

gagarine’s picture

Issue summary: View changes

Updated issue summary.

gagarine’s picture

Issue summary: View changes

Updated issue summary.

markhalliwell’s picture

Issue summary: View changes
Status: Active » Closed (duplicate)
Related issues: +#2156881: Support (optionally) the use of the jQuery Migrate Plugin

This was "fixed" by adding support for the jQuery Migrate plugin.