Only uncheck radios in same form#1007
Conversation
When selecting another radio element, only other radios in the same form should be unchecked.
|
I am almost certain this was fixed in jsdom 1.4.0. As such I'd appreciate a failing test to show that this is still an issue. /cc @Joris-van-der-Wel. |
|
Actually it might have been fixed in 1.0.0-pre.6. |
|
https://github.com/tmpvar/jsdom/blob/master/test/level2/html.js#L19814-L19833 would lead me to believe you're correct. However that's just testing when you have 2 sibling forms. If I copy & modify that test to match the jsbin, then it fails (will add that to PR in a moment) |
|
Note: I did confirm that the new test fails in master without the rest of my patch. |
|
Sweet!! We'd better merge this then :D. |
|
Here are a few quotes from WHATWG radio button group definition
html.js:1036 loops over only the descendants of the radio button group (or at least, that was my intention :) ). But it does not strictly follow the spec with nested forms. |
|
Well it would be lovely if you wanted to make it follow the spec exactly. Let me know if you're up for that and if so I'll wait on merging. |
|
Sure, but probably not before this weekend. It looks pretty involved though, iirc jsdom does not implement setting the form attribute. Following the spec strictly would require tracking form owners explicitly (instead of looping over parentNode). |
|
Oh I see, yeah that sounds like a pain. Not worth tying up this PR for. |
|
Merged as 3a75559, with 3.0.2 going out shortly! |
|
Awesome turnaround, thanks all! |
When selecting another radio element, only other radios in the same form should be unchecked.
See http://jsbin.com/daqoge/1/ as an example. This is taken pretty much directly from a test we have in React where we're ensuring we maintain browser behavior. In order to make it pass in jest we actually "fix" this in our (ancient) jsdom env by overridding the setter for
HTMLInputElement.checked(https://github.com/facebook/jest/blob/master/src/lib/jsdom-compat.js#L245-L284).I think this is the right fix but there's a lot of code I didn't read. I also didn't write a test - I'll look into that process but if anybody more familiar wants to jump in and help, that would be appreciated.