Skip to content

Commit

Permalink
fix(input): don't dirty model when input event triggered due to place…
Browse files Browse the repository at this point in the history
…holder change

Certain versions of IE inexplicably trigger an input event in response to a placeholder
being set.

It is not possible to sniff for this behaviour nicely as the event is not triggered if
the element is not attached to the document, and the event triggers asynchronously so
it is not possible to accomplish this without deferring DOM compilation and slowing down
load times.

Closes angular#2614
Closes angular#5960
  • Loading branch information
caitp committed Jan 23, 2014
1 parent e020916 commit 91b3858
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 1 deletion.
11 changes: 10 additions & 1 deletion src/ng/directive/input.js
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,8 @@ function validate(ctrl, validatorName, validity, value){
}

function textInputType(scope, element, attr, ctrl, $sniffer, $browser) {
var lastValue = element.val();
var lastPlaceholder = element[0].placeholder;
// In composition mode, users are still inputing intermediate text buffer,
// hold the listener until composition is done.
// More about composition events: https://developer.mozilla.org/en-US/docs/Web/API/CompositionEvent
Expand All @@ -420,17 +422,24 @@ function textInputType(scope, element, attr, ctrl, $sniffer, $browser) {
});
}

var listener = function() {
var listener = function(event) {
if (composing) return;
var value = element.val();

if (msie && event && event.type === 'input' && !value && value === lastValue &&
element[0].placeholder !== lastPlaceholder) {
lastPlaceholder = element[0].placeholder;
return;
}

// By default we will trim the value
// If the attribute ng-trim exists we will avoid trimming
// e.g. <input ng-model="foo" ng-trim="false">
if (toBoolean(attr.ngTrim || 'T')) {
value = trim(value);
}

lastValue = value;
if (ctrl.$viewValue !== value) {
if (scope.$$phase) {
ctrl.$setViewValue(value);
Expand Down
37 changes: 37 additions & 0 deletions test/ng/directive/inputSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,43 @@ describe('input', function() {
});
}


it('should not dirty the model on an input event in response to a placeholder change', inject(function($document) {
var $body = $document.find('body');
compileInput('<input type="text" ng-model="name" name="name" placeholder="{{placeholder}}" />');
$body.append(formElm);
var waited = false;

scope.$apply(function() {
scope.placeholder = "Test";
});
setTimeout(function() { waited = true; }, 500);

waitsFor(function() {
return waited;
}, 1000);

runs(function() {
expect(inputElm.attr('placeholder')).toBe('Test');
expect(inputElm).toBePristine();
waited = false;
scope.$apply(function() {
scope.placeholder = "Test Again";
});
setTimeout(function() { waited = true; }, 500);

waitsFor(function() {
return waited;
}, 1000);
runs(function() {
expect(inputElm.attr('placeholder')).toBe('Test Again');
expect(inputElm).toBePristine();
inputElm.remove();
});
});
}));


describe('"change" event', function() {
function assertBrowserSupportsChangeEvent(inputEventSupported) {
// Force browser to report a lack of an 'input' event
Expand Down

3 comments on commit 91b3858

@Benxamin
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did this ever get merged back into Angular.js?

@caitp
Copy link
Owner Author

@caitp caitp commented on 91b3858 May 22, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Benxamin angular@ff428e7 --- I don't believe this was ever reverted

@Benxamin
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh... It's in at 1.3.0-beta.6 yet to be released... Thank you!

Please sign in to comment.