Fixes IE11 issue with select losing focus having selected an item.

[ToreOlavKristiansen]

This pull request includes a

• [ x] Bug fix
• New feature
• Translation

The following changes were made

• Changed from calling focus on the search input element to the select element.

If this is related to an existing ticket, include a link to it as #4570 #3332.

[ToreOlavKristiansen]

I closed the pull request because one of the automated tests failed.

[ToreOlavKristiansen]

I did more testing and I don't see any issue even if the test fails. The input field might not have focus, but the select has had its focus call and thus everything works.

[jeznag]

Will this have side effects on other types of inputs? I did another PR which only modifies the behaviour for tag input as a precaution. https://github.com/select2/select2/pull/4958/files

[ToreOlavKristiansen]

$element.focus() invokes the focus function of the select2 plugin, not the "tag". This function should handle all cases so I don't think it has any side effects. We are using select elements in our solutions and the bug needs fixing for that too.

Did I understand you correctly?

[alexweissman]

It seems that there are several competing solutions to this problem:

• Focus is lost while entering tags in Internet Explorer (IE11) #4398 (comment)
• https://github.com/select2/select2/pull/4958/files
• 294e7e6

[alexweissman]

@ToreOlavKristiansen @jeznag can either of you confirm that this is still an issue in Select2 v4.0.5? We fixed a more glaring issue with focus in 4.0.4., so I'm not sure if this is just another manifestation of that problem, or a separate issue specific to IE. Thanks!

[ToreOlavKristiansen]

@alexweissman it is improved in v4.0.5, but it is still a problem for multi-select: focus is lost when selecting an item in IE11. It works fine in Chrome.

I tested v4.0.5 with this change. It fixes the problem.

[alexweissman]

Alright, I'll merge this in!

[alexweissman]

Actually, we need to fix the broken build. If you believe that the failing tests do not reflect accurately on the expected behavior, then we need to change either your code or the tests 😛

I notice that #4958 is handling the focus correctly, so maybe for your solution to work, we just need to add some logic for searchHadFocus? We can still leave out their check for isTagInput if you believe that is not actually necessary.

[ToreOlavKristiansen]

I will test some more tomorrow. In my test earlier today I did not alter base.js and it looked like Chrome was ok without it. If that is the case, I will add another pull request.

[ToreOlavKristiansen]

I tested. We need both changes. I modified to please the unit tests. I also fixed an obvious duplicate text issue in the Norwegian translation.

[alexweissman]

Awesome, thank you! Not sure what happened with the Norwegian translation, I must have performed a bad merge conflict resolution 😕

[ToreOlavKristiansen]

We use the select2 control a lot and really appreciate all the hard work put in on it. I am glad to contribute. Thanks :)

[alexweissman]

Merged into develop for the 4.0.6-rc.1 pre-release.

[alexweissman]

Actually, one more question - did you try just checking searchHadFocus instead of isTagInput?

[ToreOlavKristiansen]

I did. Then everything appears to be working fine, but the unit tests fail:

Selection containers - Inline search - updating selection does not shift the focus
Message: The search did not have focus after the selection was updated
Actual:
Expected:
at http://localhost:9999/tests/vendor/qunit-1.23.1.js:1485
at http://localhost:9999/tests/selection/search-tests.js:137
at runTest (http://localhost:9999/tests/vendor/qunit-1.23.1.js:843)
at http://localhost:9999/tests/vendor/qunit-1.23.1.js:828
at http://localhost:9999/tests/vendor/qunit-1.23.1.js:970
at process (http://localhost:9999/tests/vendor/qunit-1.23.1.js:629)
at begin (http://localhost:9999/tests/vendor/qunit-1.23.1.js:611)
at http://localhost:9999/tests/vendor/qunit-1.23.1.js:671

[alexweissman]

Ah ok, I understand. Everything is wrapped in if (searchHadFocus) anyway 👍

[alexweissman]

@ToreOlavKristiansen I'm still seeing a possibly related problem in #3168 - can you take a look?

[ToreOlavKristiansen]

@alexweissman I do get this issue in IE11. I wouldn't say it is related to the lost focus issue though.

I see that the selected items are cleared first and then IE gets the backspace key event and goes back. Should we check for a backspace key event when clearing the items and prevent it from bubbling up?

[alexweissman]

We could try that, but I'd like to know why this only seems to affect IE 😕

[ToreOlavKristiansen]

I see that having used backspace, it looks like the control has focus, but is not possible to start typing in Chrome. So the search textbox does not have focus. IE then probably figures since no input field has focus and goes back when it gets the backspace. The issue could be fixed by calling this.$element.focus(); having cleared the items. Then it is perhaps possible to start typing too.

[alexweissman]

It would seem that there are other issues related to IE losing focus too easily (#2503, for example). Maybe it has ADHD 🙂

At any rate, #3394 claims to fix this. Can you check it out?

[ToreOlavKristiansen]

No, that does not fix it.

I was able to fix it by adding this code as shown in the image:

evt.preventDefault();
var self = this;
window.setTimeout(function () {
    self.$element.focus();
}, 0);

[ToreOlavKristiansen]

The prevent default on the event fixes the IE browser back issue. The focus fixes not being able to start typing for both IE and Chrome.

[alexweissman]

Interesting...I wonder what #3394 is trying to achieve, then. Seems like it is doing something similar, but in search.js instead of allowClear.js.

[ToreOlavKristiansen]

Let me know if you want a pull request for this.

[alexweissman]

Yes, that would be great!

[ToreOlavKristiansen]

Here it is: #5161

I made the mistake of including the dist files. Will you be able to sort that out?
I don't use git often. I see that the changes I already created a pull request for are included in this new pull request, not just the allowClear.js modifications. Please let me know if I am doing something wrong here.

[alexweissman]

Thanks! Yes, it would be more helpful if the PR didn't include changes to dist. However, if they are in a separate commit, I can just cherry-pick the src commits.

[nehachaudhary]

@ToreOlavKristiansen The fix you gave in 5161 doesn't work either.