New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.prop("selected", true) does not work as expected in IE11 #2732

Closed
kennyk-peplink opened this Issue Nov 19, 2015 · 8 comments

Comments

Projects
None yet
4 participants
@kennyk-peplink

kennyk-peplink commented Nov 19, 2015

By selecting option with .prop("select", true), the behavior is not as expected under IE11

Testing with Firefox / Chrome / Safari, the behavior should be consistent,
the following example should have "Two" being selected:

http://jsfiddle.net/zksm1o5q/2

In IE, however, the result is "Four".

You may comment out Line 11, 12, and/or 14 for more test cases with expected result (last prop being selected) in other browsers, but IE11.

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Nov 19, 2015

Member

To remove a red herring here, the special characters in the values don't change the outcome. Here is a simpler case, with the last selection changing the text to show that it was able to select the right element: http://jsfiddle.net/zksm1o5q/5/

Here's a bare DOM example without jQuery that exhibits the same problem: http://jsfiddle.net/15ytbys8/

Member

dmethvin commented Nov 19, 2015

To remove a red herring here, the special characters in the values don't change the outcome. Here is a simpler case, with the last selection changing the text to show that it was able to select the right element: http://jsfiddle.net/zksm1o5q/5/

Here's a bare DOM example without jQuery that exhibits the same problem: http://jsfiddle.net/15ytbys8/

@timmywil

This comment has been minimized.

Show comment
Hide comment
@timmywil

timmywil Nov 19, 2015

Member

This is a browser bug, but perhaps we should work around it in a propHook that sets all other option elements' selected values to false?

Member

timmywil commented Nov 19, 2015

This is a browser bug, but perhaps we should work around it in a propHook that sets all other option elements' selected values to false?

@timmywil timmywil added the Attributes label Nov 19, 2015

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Nov 19, 2015

Member

Does that fix the problem? Is it only a bug with options that exist in the select already, or would we also need to guard against ones appended from elsewhere?

This seems to work on Edge so it's only IE11 (and older IEs) affected.

In any event it seems like that might really be a performance killer, especially on selects with a lot of options. I guess it could be limited by a feature detect to only IE and only when changing the selected property, but silently trying to cure it might lead to a real performance trap. I'm also wondering if properties like readonly or disabled misbehave the same way?

Member

dmethvin commented Nov 19, 2015

Does that fix the problem? Is it only a bug with options that exist in the select already, or would we also need to guard against ones appended from elsewhere?

This seems to work on Edge so it's only IE11 (and older IEs) affected.

In any event it seems like that might really be a performance killer, especially on selects with a lot of options. I guess it could be limited by a feature detect to only IE and only when changing the selected property, but silently trying to cure it might lead to a real performance trap. I'm also wondering if properties like readonly or disabled misbehave the same way?

@timmywil

This comment has been minimized.

Show comment
Hide comment
@timmywil

timmywil Nov 19, 2015

Member

What about setting selectedIndex on the select element instead of setting all the selected properties?

Member

timmywil commented Nov 19, 2015

What about setting selectedIndex on the select element instead of setting all the selected properties?

@timmywil

This comment has been minimized.

Show comment
Hide comment
@timmywil

timmywil Nov 19, 2015

Member

That wouldn't be a big perf hit – or code size hit – and it seems to work: http://jsfiddle.net/timmywil/15ytbys8/1/.

Member

timmywil commented Nov 19, 2015

That wouldn't be a big perf hit – or code size hit – and it seems to work: http://jsfiddle.net/timmywil/15ytbys8/1/.

@timmywil

This comment has been minimized.

Show comment
Hide comment
@timmywil

timmywil Nov 19, 2015

Member

I don't think disabled or readonly would have the same problem because multiple options can be disabled and readonly.

Member

timmywil commented Nov 19, 2015

I don't think disabled or readonly would have the same problem because multiple options can be disabled and readonly.

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Nov 19, 2015

Member

Yeah if that works it sounds pretty good. So if we're setting selected on an option and it's already in a select that is not multiple, set the selectedIndex property. #youmightnotneedjquery

Member

dmethvin commented Nov 19, 2015

Yeah if that works it sounds pretty good. So if we're setting selected on an option and it's already in a select that is not multiple, set the selectedIndex property. #youmightnotneedjquery

@timmywil timmywil modified the milestones: 3.0.1, 3.0.0 Jan 14, 2016

@timmywil timmywil self-assigned this Jan 15, 2016

timmywil added a commit to timmywil/jquery that referenced this issue Jan 15, 2016

@timmywil timmywil closed this in 780cac8 Jan 19, 2016

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Mar 6, 2016

Member

This is a non-breaking change; should we backport it to 1.12.2 & 2.2.2?

Member

mgol commented Mar 6, 2016

This is a non-breaking change; should we backport it to 1.12.2 & 2.2.2?

@mgol mgol removed the Has Pull Request label Mar 6, 2016

@timmywil timmywil modified the milestones: 1.12.2/2.2.2, 3.0.0 Mar 7, 2016

@lock lock bot locked as resolved and limited conversation to collaborators Jun 18, 2018

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