emberjs / ember.js

Ember.js - A JavaScript framework for creating ambitious web applications
https://emberjs.com
MIT License
22.46k stars 4.21k forks source link

Possible radio input issue glimmer 2 #14712

Closed jcbvm closed 2 years ago

jcbvm commented 7 years ago

I just want to share that I found a little problem with glimmer 2.

In IE 11 I see some strange things happening to radio inputs. If the value is a computed property, the value gets set to "on" by IE when something on the page changes. A deeper search to this ends up with IE setting the value to "on" when input.type gets set.

So my guess is that a rerender causing the input.type to be set to 'radio', causing the value to change. Normally you can fix this by setting the value right after the type has been set. I'm not sure yet if this is the case in glimmer 2, does anyone knows more about this?

I tested this on IE, but from searching I see FF may also be causing this behaviour.

UPDATE

Here a jsbin showing the 'bug', if you run this in IE 11, and you click on the radio button, you will see the value changing in "on":

http://emberjs.jsbin.com/basoxirumo/1/edit?html,js,output

adam-knights commented 7 years ago

Can confirm that i also see this on ie 11 (windows 10).

dougestey commented 6 years ago

Can also confirm, Windows 10/IE11.

runspired commented 6 years ago

@chadhietala did glimmer ever do anything to address this order-of-operations bug for IE11?

Serabe commented 6 years ago

@runspired type attribute is set last to avoid a bug in input[type="range] https://github.com/glimmerjs/glimmer-vm/blob/ad2eb654b99d4831ee7313f6016b69778c7cd0e1/packages/@glimmer/compiler/lib/template-compiler.ts#L91-L100

jherdman commented 6 years ago

We have a reproduction app now: https://github.com/PrecisionNutrition/input-helper-demo/tree/ie11

jherdman commented 6 years ago

And an even simpler reproduction https://jsbin.com/foxoqiwola/1/edit. It looks like having type set last is the culprit.

pixelhandler commented 5 years ago

@adam-knights @dougestey @jcbvm @jherdman @lepolt @runspired is this still an issue, perhaps we should close or create a new reproduction of this, what do you think?

jherdman commented 5 years ago

Very much still an issue. Does the JSBin example not count?

pixelhandler commented 5 years ago

@jherdman well the original jsbin is using old version of Ember and ember-twiddle doesn't run in IE. I see the https://github.com/PrecisionNutrition/input-helper-demo/blob/ie11/package.json example perhaps it can be update to a current release of Ember?

jherdman commented 5 years ago

@pixelhandler done, and done!

tomdale commented 5 years ago

@Serabe @jherdman So am I understanding correctly that this issue with radio buttons is exacerbated by the fix for input[type="range"], since we always move setting the type attribute to the end?

jherdman commented 5 years ago

@tomdale yeah, probably. The lesson for me is that various implementations do NOT treat attribute insertion as irrelevant. The good news is that (AFAIK) the range and radio elements are the only ones that we seem to have run into.... that I know of :)

jherdman commented 3 years ago

IMHO we can close this.

sandstrom commented 2 years ago

I'm doing some issue gardening 🌱🌿 🌷 and came upon this issue. Since it's quite old I just wanted to ask if this is still relevant? If it isn't, maybe we can close this issue?

By closing some old issues we reduce the list of open issues to a more manageable set.

jherdman commented 2 years ago

IMHO yes