-
Notifications
You must be signed in to change notification settings - Fork 27.4k
fix(jqLite): Firefox validation trigger #12106
Conversation
@@ -671,7 +671,10 @@ forEach({ | |||
} | |||
return element.value; | |||
} | |||
element.value = value; | |||
// Workaround for Firefox validation #12102. | |||
if (element.value != value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the !=
intentional ? It can lead to bugs in corner cases.
E.g. when value === ''
and one calls elem.val(0)
, they would expect elem.val() === '0'
, but since 0 == ''
, nothing will happen (I think).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
element.value
is always a string. If the value we are going to set it to will be transformed to the same string, then do not copy the value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but value
can be anything the user passes (e.g. 0
).
If element.value === ''
and value === 0
, I would expect the element's value to be set to '0'
, but because '' != 0
evaluates to false, the element's value will remain ''
, right ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rules for transforming to string are not the same as the rules for !=
comparison.
Maybe you should compare element.value
to String(value)
or something ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm...although that is not accurate for value === null
, because String(null) === 'null'
, but element.value = null ==> element.value === ''
, so that need to be taken into account as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should just be on the safe side and do !==
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But, does it solve the initial Firefox issue :) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It solves it as long as the initial value of ngModel
is not the same as the input[value]
(in most cases we expect that input[value]
to be an empty string, so yes, just doing !==
should be fine... famous last words)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, since https://github.com/angular/angular.js/blob/master/src/ng/directive/input.js#L1128 takes care of the empty values (null, undefined), it should be fine :)
So, using !==
sounds like the simplest (aka best) way out...
If we decide that we want this change, would it be possible to make the change in a "higher" level (i.e. somewhere in |
It is possible to do this here https://github.com/angular/angular.js/blob/master/src/ng/directive/input.js#L1128 |
Do not trigger Firefox validation on form initialization. - Do not set a value to an <input> field if the field already has the same value Closes angular#12102
landed as e742316 |
Add a test that checks that an <input> value is not set when the value is equal to the current value
Add a test that checks that an <input> value is not set when the value is equal to the current value Closes #12592
Add a test that checks that an <input> value is not set when the value is equal to the current value Closes angular#12592
Add a test that checks that an <input> value is not set when the value is equal to the current value Closes #12592
Do not trigger Firefox validation on form initialization.
Closes #12102