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

firefox hack for onSelect is leaky without a noop onKeyUp #7222

Closed
ianstormtaylor opened this issue Jul 8, 2016 · 1 comment
Closed

firefox hack for onSelect is leaky without a noop onKeyUp #7222

ianstormtaylor opened this issue Jul 8, 2016 · 1 comment

Comments

@ianstormtaylor
Copy link

ianstormtaylor commented Jul 8, 2016

Do you want to request a feature or report a bug?

Bug.

What is the current behavior?

Right now, React provides a cross-browser workaround for the onSelect event. In Firefox, the selectionchange event doesn't fire, so React hacks around this by evaluating the selection after both keydown and keyup, which is smart!

However, if you haven't added an onKeyUp handler to your component, the keyup part of the selection listening hack for Firefox won't ever fire. Which means if you happen to be doing something in onKeyDown that changes the selection, it won't be caught by keyup as expected, such that the next time keydown fires, the onSelect handler will be triggered (via the keydown part of the hack), since React thinks the selection has changed.

Example:

  • You place your cursor at the end of a sentence.
    • onSelect fires thanks to mouseup.
    • render updates the selection.
  • You backspace the last . character:
    • onKeyDown you handle deleting the character, and preventing default.
    • onSelect doesn't fire, because on change hasn't taken place yet, as expected.
    • render updates the selection.
  • You backspace another character:
    • onKeyDown you handle deleting another character.
    • onSelect fires thanks to onkeydown, but its internal state is comparing it to the selection before it was updated by render, so it is out of date. Not expected.

If you add a noop handler for onKeyUp, this problem goes away, since onSelect will always fire after rendering thanks to keyup, and then it will never fire incorrectly due to keydown.

An example is in Draft.js, this onKeyUp handler actually doesn't do anything. Except without, the onSelect handling wouldn't stay in sync in Firefox.

What is the expected behavior?

Ideally I'd think that the keyup event hack would still fire even without having to have the developer add a onKeyUp noop to their contentEditable component?

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

React 15 in Firefox.

@sophiebits
Copy link
Collaborator

Thanks for the detailed report @ianstormtaylor! Sorry I didn't get to this until now.

This should be an easy fix – just adding onKeyUp below onKeyDown here:

zpao pushed a commit that referenced this issue Sep 15, 2016
Fixes #7222.
(cherry picked from commit 869cb05)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants