$().prop("tabIndex") returns wrong value (JSBin example included) #2647

Closed
aschmalzhaf opened this Issue Oct 13, 2015 · 5 comments

Comments

Projects
None yet
4 participants
@aschmalzhaf

Hello,

$(...).prop("tabIndex") behaves differently in JQuery 1.11.3 and 2.1.4 when Internet Explorer 11 is used.
This is true at least for images (dom nodes of type ) that doesn't have an attribute "tabindex" explicitly given.

Please see also the playground example here:
http://jsbin.com/herehipoke/1/edit?html,output

When changing the URL of the loaded jQuery version, you can see that the tabIndex is different ("0" vs. "-1").

This is because of changes in attributes/porp.js file...
The propHook for tabIndex is different:

The current code line (master and 2.1 stable) show the code of propHooks - tabIndex as follows:

get: function( elem ) {
return elem.hasAttribute( "tabindex" ) || rfocusable.test( elem.nodeName ) || elem.href ?
elem.tabIndex : -1;
}

1.11.1 looks like that:

get: function( elem ) {
var tabindex = jQuery.find.attr( elem, "tabindex" );
return tabindex ?
parseInt( tabindex, 10 ) :
rfocusable.test( elem.nodeName ) || rclickable.test( elem.nodeName ) && elem.href ? 0 : -1;
}

I've seen that jQuery Compat 3.0 Alpha uses the 1.11 logic again. Maybe it would make sense to move that kind of logic also into jQuery 2/3 as Internet Explorers are still supprted (at least the newer ones starting from 9 upwards).

Best regards,
Alexander

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Oct 13, 2015

Member

Thanks for the report!

jQuery Compat 3.0 is the continuation of the jQuery 1.x line so nothing changed here; jQuery 3.0 is a continuation of the jQuery 2.x line and the code there is the same as in 2.x.

It does seem the compat code is still needed on the main branch as IE 11 is still broken here. Fortunately it seems fixed in Edge 12.

We'll most likely not fix it in jQuery 2.1.x (there are many bug fixes people would like to have fixed there, we can't backport all of them) but it'd be good to fix it for 3.0.0.

Member

mgol commented Oct 13, 2015

Thanks for the report!

jQuery Compat 3.0 is the continuation of the jQuery 1.x line so nothing changed here; jQuery 3.0 is a continuation of the jQuery 2.x line and the code there is the same as in 2.x.

It does seem the compat code is still needed on the main branch as IE 11 is still broken here. Fortunately it seems fixed in Edge 12.

We'll most likely not fix it in jQuery 2.1.x (there are many bug fixes people would like to have fixed there, we can't backport all of them) but it'd be good to fix it for 3.0.0.

@mgol mgol added this to the 3.0.0 milestone Oct 13, 2015

@mgol mgol added the Attributes label Oct 13, 2015

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Oct 13, 2015

Member

@aschmalzhaf Would you like to take a shot at a PR? jQuery Compat 3.0 alpha code lies on the compat branch, jQuery 3.0 alpha code lies on master; you'd need to backport the compat code into the master branch; it's in the src/attributes/prop.js file.

Member

mgol commented Oct 13, 2015

@aschmalzhaf Would you like to take a shot at a PR? jQuery Compat 3.0 alpha code lies on the compat branch, jQuery 3.0 alpha code lies on master; you'd need to backport the compat code into the master branch; it's in the src/attributes/prop.js file.

Queeniebee added a commit to Queeniebee/jquery that referenced this issue Oct 18, 2015

@mgol mgol closed this in c752a50 Oct 18, 2015

@mgol mgol added the 2.x-only label Oct 18, 2015

gibson042 added a commit that referenced this issue Oct 25, 2015

Attributes: fix tabIndex on <img> in IE11
Fixes gh-2647
Closes gh-2664

(cherry picked from commit c752a50)

Conflicts:
	src/attributes/prop.js
@markelog

This comment has been minimized.

Show comment
Hide comment
@markelog

markelog Dec 22, 2015

Member

@mzgol why this was ported to compat if there is master-only label?

Member

markelog commented Dec 22, 2015

@mzgol why this was ported to compat if there is master-only label?

@markelog markelog removed the 2.x-only label Dec 22, 2015

@markelog

This comment has been minimized.

Show comment
Hide comment
@markelog

markelog Dec 22, 2015

Member

Label removed

Member

markelog commented Dec 22, 2015

Label removed

@dmethvin dmethvin modified the milestones: 1.12/2.2, 3.0.0 Jan 8, 2016

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Jan 14, 2016

Member

Only a test was added, nothing needed to be fixed in 1.x. I'm adding the label again.

Member

mgol commented Jan 14, 2016

Only a test was added, nothing needed to be fixed in 1.x. I'm adding the label again.

@mgol mgol added the 2.x-only label Jan 14, 2016

@cssmagic cssmagic referenced this issue in cssmagic/ChangeLog May 18, 2016

Open

jQuery #5

@jquery jquery locked as resolved and limited conversation to collaborators Jun 19, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.