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

Controlled <select> doesn't update in Firefox when also setting state in another event #12584

Closed
benclive opened this issue Apr 9, 2018 · 7 comments

Comments

@benclive
Copy link

benclive commented Apr 9, 2018

Reporting a Bug

What is the current behavior?
When selecting a value in a React controlled select box in Firefox, the selection doesn't change. This occurs when a MouseDown handler also generates a state change.

It seems to occur when the MouseDown handler sets the state and re-renders the dropdown before the onChange event handler fires. The onChange is then triggered with the old select value because React provided the old state during the re-render.

CodePen (modified example from the Docs): https://codepen.io/anon/pen/zWyJqg?editors=0010
Steps to reproduce:

  1. Run example on Firefox
  2. Open the dropdown and select a different value (e.g. Grapefruit)
  3. Observe that the new value is not selected.

What is the expected behavior?
Select dropdown is updated and the value is available in React.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
Tested on Firefox v59.0.2 (Development Edition and Normal) on OSX using React v16.3.1 and v16.1.1
Also encountered on Windows but I'm not sure of the exact versions.
Tested on Chrome and everything works as intended on OSX and Windows.

@aweary
Copy link
Contributor

aweary commented Apr 9, 2018

Thanks for the report and repro @benclive! Looks like a bug to me, we'll try to investigate it soon. If you'd like to try and find the root cause and send in a fix we'd greatly appreciate that too 🙂

@faefelipe
Copy link

faefelipe commented Apr 11, 2018

I believe that the following issue occurs because onMouseDown Event
should not be in the div.
If we remove that events from div the code will work fine on both Browsers.

@skiritsis
Copy link
Contributor

@faefelipe This is a workaround rather than a solution.
@benclive I believe the issue lies in the fact that Firefox triggers 3 events (onmousedown, onmousedown, onchange) instead of 2 (onmousedown, onchange) like Chrome does as @nuragic noticed as well.

In your example, because you have the following line in 'handleMouseDown':
this.setState({ shiftEnabled: event.shiftKey });
You re-render the component in Firefox(since an additional event is being triggered) that's why it seems like the value is not getting updated.
If you comment the line above it works as expected in both browsers.

@jacksongabbard
Copy link

jacksongabbard commented Apr 20, 2018

Was just having a little poke at this. I believe @skiritsis is on the right track. There are two MouseDown events getting fired. First, a MouseDown fires when the <select /> is initially interacted with and the browser shows the list of values. Second, a MouseDown again fires when any specific value is chosen from that modal list. If you throw a console.log into that handleMouseDown function to output the value of the select list, it will show the correct value prior to the setState() call.

I'd argue that this is not a bug. Firefox is giving you a more nuanced view of the state changes in the page than Chrome does. If you wanted to leverage this additional information, you could change the code to this:

handleMouseDown(event) {
  this.setState({ 
      value: event.target.value,
      shiftEnabled: event.shiftKey 
  });
}

However, this will result in the wrong thing being written to the state when "Submit" is clicked because, as @faefelipe points out, you're attaching the onMouseDown to the <div /> parent of the form. In general, MouseDown events are not a great way to approach this because touch screen devices don't have mice and only have simulated mouse events.

I really don't think anything within React needs to change for this error report. It's application specific behaviour that is based on fairly nuanced browser-dependent behaviour. We could pin one vendor to the other and force them to behave the same way, but that is complexity that has a cost over time.

@dmmulroy
Copy link

dmmulroy commented Jul 3, 2018

I'm having this issue too with a select input. However, instead of attaching my onMouseUp (mouse up in my case) to a div in my component, I have to attach it to the window. I do this because I'm working with <canvas> and if a user drags/mouse down + mouse moves off the chart I need to capture that still and don't have access to the parent that renders the canvas. In @benclive's example you can add event.persist() to the onMouseDown event handler and everything will work as expected. In my case, a SyntheticEvent is not provided to window event handlers so I can't persist. Looking for any help here as we have resorted to putting a onClick handler on the <option></option> components we render.

@gaearon
Copy link
Collaborator

gaearon commented Aug 7, 2018

Closing per #12584 (comment) — doesn't seem like React could make that choice for you.

@gaearon gaearon closed this as completed Aug 7, 2018
@gaearon gaearon removed the Type: Bug label Aug 7, 2018
@googol7
Copy link

googol7 commented Sep 21, 2018

@jacksongabbard We do use MouseDown for a desktop application where a feature only makes sense when used with the mouse. We have a SelectField component that behaved differently on Firefox when used within another component that had MouseDown attached to an outer <div>.

We think that the component SelectField should be robust no matter how it’s being used. That’s why we prevent such problems like this now in SelectField:

const onMouseDown = event => {
    event.stopPropagation()
}

const onMouseUp = event => {
    event.stopPropagation()
}

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

8 participants