Skip to content
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

autocomplete: fix for IE scrolling issues #1785

Closed
wants to merge 2 commits into from
Closed

autocomplete: fix for IE scrolling issues #1785

wants to merge 2 commits into from

Conversation

AttackTheDarkness
Copy link

IE11 and scrolling autocompletes didn't get along great; this should help fix
their relationship.

When you click on an autocomplete scrollbar in IE11, the menu temporarily
gains focus, which caused a couple problems.

  1. Depending on how long you clicked, the dropdown could close.

  2. Scrolling down by clicking the scrollbar's down arrow would misbehave. The
    list would pop back up to the top with the first item selected.

We can fix both problems by modifying the focus/blur handling a bit.

  1. There is a flag to instruct the control to ignore blurs, but it was getting
    cleared too quickly; when the code refocused the input after it was blurred,
    IE would send another blur event, which wasn't getting ignored and would
    close the dropdown. We now wait for the focus/blur pair to process before
    clearing the flag.

  2. We remove the tabindex from the dropdown menu, which prevents menu's focus
    handler from firing. When you focus a menu, it will select the first menu item
    if none are selected. Selecting a menu item will scroll it into view if it's
    not visible. This combination of behaviors was causing the strange behavior
    when attempting to scroll down.

I couldn't figure out a way to write a unit test for this, since it's IE only
and seems to require user interaction. You can verify the previous behavior
(and the fix) on demos/autocomplete/maxheight.html

Fixes #9638

IE11 and scrolling autocompletes didn't get along great; this should help fix
their relationship.

When you click on an autocomplete scrollbar in IE11, the menu temporarily
gains focus, which caused a couple problems.

1. Depending on how long you clicked, the dropdown could close.

2. Scrolling down by clicking the scrollbar's down arrow would misbehave. The
list would pop back up to the top with the first item selected.

We can fix both problems by modifying the focus/blur handling a bit.

1. There is a flag to instruct the control to ignore blurs, but it was getting
cleared too quickly; when the code refocused the input after it was blurred,
IE would send *another* blur event, which wasn't getting ignored and would
close the dropdown. We now wait for the focus/blur pair to process before
clearing the flag.

2. We remove the tabindex from the dropdown menu, which prevents menu's focus
handler from firing. When you focus a menu, it will select the first menu item
if none are selected. Selecting a menu item will scroll it into view if it's
not visible. This combination of behaviors was causing the strange behavior
when attempting to scroll down.

I couldn't figure out a way to write a unit test for this, since it's IE only
and seems to require user interaction. You can verify the previous behavior
(and the fix) on `demos/autocomplete/maxheight.html`

Fixes #9638
@jsf-clabot
Copy link

jsf-clabot commented Jan 12, 2017

CLA assistant check
All committers have signed the CLA.

// and ignored the blur event because of the cancelBlur flag set above. So
// we restore focus to ensure that the menu closes properly based on the user's
// next actions.
// Support: IE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't use unversioned support comments like this. Presumably you're hitting a different scenario (not right clicking), so the following comment explaining the support comment isn't right anymore.

// already received and ignored the blur event because of the cancelBlur flag
// set above. So we restore focus to ensure that the menu closes properly based
// on the user's next actions.
// Note that the focus event can itself raise another blur event, so we need
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be around the call to _delay.

@scottgonzalez
Copy link
Member

Does this only affect IE and not Edge?

@AttackTheDarkness
Copy link
Author

I'm not sure if Edge is affected... We found the bug in IE11, and I have easy access to that browser. I'll DL a VM and give it a whirl, and amend the comment as appropriate.

IE11 and Edge will not prevent focus when you `preventDefault` on a mousedown
events; this was causing some undesireable behavior:

1. Depending on how long you clicked, the dropdown could close (IE11)

2. Scrolling down by clicking the scrollbar's down arrow would misbehave. The
list would pop back up to the top with the first item selected. (IE11, Edge)

Both these problems are fixed by making the dropdown un-focusable (by setting
the IE/Edge-only `unselectable` attribute to `on`).

I couldn't figure out a way to write a unit test for this, since it's IE only
and seems to require user interaction. You can verify the previous behavior
(and the fix) on demos/autocomplete/maxheight.html

Fixes #9638
@AttackTheDarkness
Copy link
Author

@scottgonzalez took a different tack on the fix, and am happier with the results. Let me know what you think. This was a problem in Edge too, by the way. I've verified the fix in both browsers.

@scottgonzalez
Copy link
Member

Thanks. It's nice to finally have this working properly in all browsers. Managing focus in IE has been a huge pain for us over the years.

@AttackTheDarkness AttackTheDarkness deleted the autcomplete_scroll_ie branch August 3, 2017 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants