Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix(inputs): ignoring input events in IE caused by placeholder changes #9697

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions src/ng/directive/input.js
Original file line number Diff line number Diff line change
@@ -954,15 +954,6 @@ function baseInputType(scope, element, attr, ctrl, $sniffer, $browser) {
var value = element.val(),
event = ev && ev.type;

// IE (11 and under) seem to emit an 'input' event if the placeholder value changes.
// We don't want to dirty the value when this happens, so we abort here. Unfortunately,
// IE also sends input events for other non-input-related things, (such as focusing on a
// form control), so this change is not entirely enough to solve this.
if (msie && (ev || noevent).type === 'input' && element[0].placeholder !== placeholder) {
placeholder = element[0].placeholder;
return;
}

// By default we will trim the value
// If the attribute ng-trim exists we will avoid trimming
// If input type is 'password', the value is never trimmed
6 changes: 5 additions & 1 deletion src/ng/directive/ngEventDirs.js
Original file line number Diff line number Diff line change
@@ -53,7 +53,11 @@ forEach(
return {
restrict: 'A',
compile: function($element, attr) {
var fn = $parse(attr[directiveName]);
// We expose the powerful $event object on the scope that provides access to the Window,
// etc. that isn't protected by the fast paths in $parse. We explicitly request better
// checks at the cost of speed since event handler expressions are not executed as
// frequently as regular change detection.
var fn = $parse(attr[directiveName], /* interceptorFn */ null, /* expensiveChecks */ true);
return function ngEventHandler(scope, element) {
element.on(eventName, function(event) {
var callback = function() {
53 changes: 33 additions & 20 deletions src/ng/parse.js
Original file line number Diff line number Diff line change
@@ -852,7 +852,8 @@ function setter(obj, path, setValue, fullExp) {
return setValue;
}

var getterFnCache = createMap();
var getterFnCacheDefault = createMap();
var getterFnCacheExpensive = createMap();

function isPossiblyDangerousMemberName(name) {
return name == 'constructor';
@@ -863,7 +864,7 @@ function isPossiblyDangerousMemberName(name) {
* - http://jsperf.com/angularjs-parse-getter/4
* - http://jsperf.com/path-evaluation-simplified/7
*/
function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp) {
function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, expensiveChecks) {
ensureSafeMemberName(key0, fullExp);
ensureSafeMemberName(key1, fullExp);
ensureSafeMemberName(key2, fullExp);
@@ -872,11 +873,11 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp) {
var eso = function(o) {
return ensureSafeObject(o, fullExp);
};
var eso0 = isPossiblyDangerousMemberName(key0) ? eso : identity;
var eso1 = isPossiblyDangerousMemberName(key1) ? eso : identity;
var eso2 = isPossiblyDangerousMemberName(key2) ? eso : identity;
var eso3 = isPossiblyDangerousMemberName(key3) ? eso : identity;
var eso4 = isPossiblyDangerousMemberName(key4) ? eso : identity;
var eso0 = (expensiveChecks || isPossiblyDangerousMemberName(key0)) ? eso : identity;
var eso1 = (expensiveChecks || isPossiblyDangerousMemberName(key1)) ? eso : identity;
var eso2 = (expensiveChecks || isPossiblyDangerousMemberName(key2)) ? eso : identity;
var eso3 = (expensiveChecks || isPossiblyDangerousMemberName(key3)) ? eso : identity;
var eso4 = (expensiveChecks || isPossiblyDangerousMemberName(key4)) ? eso : identity;

return function cspSafeGetter(scope, locals) {
var pathVal = (locals && locals.hasOwnProperty(key0)) ? locals : scope;
@@ -911,23 +912,25 @@ function getterFnWithEnsureSafeObject(fn, fullExpression) {
}

function getterFn(path, options, fullExp) {
var expensiveChecks = options.expensiveChecks;
var getterFnCache = (expensiveChecks ? getterFnCacheExpensive : getterFnCacheDefault);
var fn = getterFnCache[path];

if (fn) return fn;


var pathKeys = path.split('.'),
pathKeysLength = pathKeys.length;

// http://jsperf.com/angularjs-parse-getter/6
if (options.csp) {
if (pathKeysLength < 6) {
fn = cspSafeGetterFn(pathKeys[0], pathKeys[1], pathKeys[2], pathKeys[3], pathKeys[4], fullExp);
fn = cspSafeGetterFn(pathKeys[0], pathKeys[1], pathKeys[2], pathKeys[3], pathKeys[4], fullExp, expensiveChecks);
} else {
fn = function cspSafeGetter(scope, locals) {
var i = 0, val;
do {
val = cspSafeGetterFn(pathKeys[i++], pathKeys[i++], pathKeys[i++], pathKeys[i++],
pathKeys[i++], fullExp)(scope, locals);
pathKeys[i++], fullExp, expensiveChecks)(scope, locals);

locals = undefined; // clear after first iteration
scope = val;
@@ -937,15 +940,18 @@ function getterFn(path, options, fullExp) {
}
} else {
var code = '';
var needsEnsureSafeObject = false;
if (expensiveChecks) {
code += 's = eso(s, fe);\nl = eso(l, fe);\n';
}
var needsEnsureSafeObject = expensiveChecks;
forEach(pathKeys, function(key, index) {
ensureSafeMemberName(key, fullExp);
var lookupJs = (index
// we simply dereference 's' on any .dot notation
? 's'
// but if we are first then we check locals first, and if so read it first
: '((l&&l.hasOwnProperty("' + key + '"))?l:s)') + '.' + key;
if (isPossiblyDangerousMemberName(key)) {
if (expensiveChecks || isPossiblyDangerousMemberName(key)) {
lookupJs = 'eso(' + lookupJs + ', fe)';
needsEnsureSafeObject = true;
}
@@ -1030,15 +1036,20 @@ function getValueOf(value) {
* service.
*/
function $ParseProvider() {
var cache = createMap();
var cacheDefault = createMap();
var cacheExpensive = createMap();

var $parseOptions = {
csp: false
};


this.$get = ['$filter', '$sniffer', function($filter, $sniffer) {
$parseOptions.csp = $sniffer.csp;
var $parseOptions = {
csp: $sniffer.csp,
expensiveChecks: false
},
$parseOptionsExpensive = {
csp: $sniffer.csp,
expensiveChecks: true
};

function wrapSharedExpression(exp) {
var wrapped = exp;
@@ -1055,13 +1066,14 @@ function $ParseProvider() {
return wrapped;
}

return function $parse(exp, interceptorFn) {
return function $parse(exp, interceptorFn, expensiveChecks) {
var parsedExpression, oneTime, cacheKey;

switch (typeof exp) {
case 'string':
cacheKey = exp = exp.trim();

var cache = (expensiveChecks ? cacheExpensive : cacheDefault);
parsedExpression = cache[cacheKey];

if (!parsedExpression) {
@@ -1070,8 +1082,9 @@ function $ParseProvider() {
exp = exp.substring(2);
}

var lexer = new Lexer($parseOptions);
var parser = new Parser(lexer, $filter, $parseOptions);
var parseOptions = expensiveChecks ? $parseOptionsExpensive : $parseOptions;
var lexer = new Lexer(parseOptions);
var parser = new Parser(lexer, $filter, parseOptions);
parsedExpression = parser.parse(exp);

if (parsedExpression.constant) {
4 changes: 3 additions & 1 deletion src/ng/sniffer.js
Original file line number Diff line number Diff line change
@@ -67,7 +67,9 @@ function $SnifferProvider() {
// IE9 implements 'input' event it's so fubared that we rather pretend that it doesn't have
// it. In particular the event is not fired when backspace or delete key are pressed or
// when cut operation is performed.
if (event == 'input' && msie == 9) return false;
// IE10+ implements 'input' event but it erroneously fires under various situations,
// e.g. when placeholder changes, or a form is focused.
if (event === 'input' && msie <= 11) return false;

if (isUndefined(eventSupport[event])) {
var divElm = document.createElement('div');
2 changes: 1 addition & 1 deletion src/ngScenario/dsl.js
Original file line number Diff line number Diff line change
@@ -199,7 +199,7 @@ angular.scenario.dsl('binding', function() {
*/
angular.scenario.dsl('input', function() {
var chain = {};
var supportInputEvent = 'oninput' in document.createElement('div') && msie != 9;
var supportInputEvent = 'oninput' in document.createElement('div') && !(msie <= 11)

chain.enter = function(value, event) {
return this.addFutureAction("input '" + this.name + "' enter '" + value + "'",
192 changes: 179 additions & 13 deletions test/ng/directive/inputSpec.js
Original file line number Diff line number Diff line change
@@ -1388,22 +1388,188 @@ describe('input', function() {
expect(scope.name).toEqual('caitp');
});

it('should not dirty the model on an input event in response to a placeholder change', inject(function($sniffer) {
if (msie && $sniffer.hasEvent('input')) {
compileInput('<input type="text" ng-model="name" name="name" />');
inputElm.attr('placeholder', 'Test');
browserTrigger(inputElm, 'input');
describe("IE placeholder input events", function() {
//IE fires an input event whenever a placeholder visually changes, essentially treating it as a value
//Events:
// placeholder attribute change: *input*
// focus (which visually removes the placeholder value): focusin focus *input*
// blur (which visually creates the placeholder value): focusout *input* blur
//However none of these occur if the placeholder is not visible at the time of the event.
//These tests try simulate various scenerios which do/do-not fire the extra input event

it('should not dirty the model on an input event in response to a placeholder change', function() {
if (msie) {
compileInput('<input type="text" placeholder="Test" attr-capture ng-model="unsetValue" name="name" />');
browserTrigger(inputElm, 'input');
expect(inputElm.attr('placeholder')).toBe('Test');
expect(inputElm).toBePristine();

attrs.$set('placeholder', '');
browserTrigger(inputElm, 'input');
expect(inputElm.attr('placeholder')).toBe('');
expect(inputElm).toBePristine();

attrs.$set('placeholder', 'Test Again');
browserTrigger(inputElm, 'input');
expect(inputElm.attr('placeholder')).toBe('Test Again');
expect(inputElm).toBePristine();

attrs.$set('placeholder', undefined);
browserTrigger(inputElm, 'input');
expect(inputElm.attr('placeholder')).toBe(undefined);
expect(inputElm).toBePristine();

changeInputValueTo('foo');
expect(inputElm).toBeDirty();
}
});

expect(inputElm.attr('placeholder')).toBe('Test');
expect(inputElm).toBePristine();
it('should not dirty the model on an input event in response to a interpolated placeholder change', inject(function($rootScope) {
if (msie) {
compileInput('<input type="text" placeholder="{{ph}}" ng-model="unsetValue" name="name" />');
browserTrigger(inputElm, 'input');
expect(inputElm).toBePristine();

inputElm.attr('placeholder', 'Test Again');
browserTrigger(inputElm, 'input');
$rootScope.ph = 1;
$rootScope.$digest();
browserTrigger(inputElm, 'input');
expect(inputElm).toBePristine();

expect(inputElm.attr('placeholder')).toBe('Test Again');
expect(inputElm).toBePristine();
}
}));
$rootScope.ph = "";
$rootScope.$digest();
browserTrigger(inputElm, 'input');
expect(inputElm).toBePristine();

changeInputValueTo('foo');
expect(inputElm).toBeDirty();
}
}));

it('should dirty the model on an input event in response to a placeholder change while in focus', inject(function($rootScope) {
if (msie) {
$rootScope.ph = 'Test';
compileInput('<input type="text" ng-attr-placeholder="{{ph}}" ng-model="unsetValue" name="name" />');
expect(inputElm).toBePristine();

browserTrigger(inputElm, 'focusin');
browserTrigger(inputElm, 'focus');
browserTrigger(inputElm, 'input');
expect(inputElm.attr('placeholder')).toBe('Test');
expect(inputElm).toBePristine();

$rootScope.ph = 'Test Again';
$rootScope.$digest();
expect(inputElm).toBePristine();

changeInputValueTo('foo');
expect(inputElm).toBeDirty();
}
}));

it('should not dirty the model on an input event in response to a ng-attr-placeholder change', inject(function($rootScope) {
if (msie) {
compileInput('<input type="text" ng-attr-placeholder="{{ph}}" ng-model="unsetValue" name="name" />');
expect(inputElm).toBePristine();

$rootScope.ph = 1;
$rootScope.$digest();
browserTrigger(inputElm, 'input');
expect(inputElm).toBePristine();

$rootScope.ph = "";
$rootScope.$digest();
browserTrigger(inputElm, 'input');
expect(inputElm).toBePristine();

changeInputValueTo('foo');
expect(inputElm).toBeDirty();
}
}));

it('should not dirty the model on an input event in response to a focus', inject(function($sniffer) {
if (msie) {
compileInput('<input type="text" placeholder="Test" ng-model="unsetValue" name="name" />');
browserTrigger(inputElm, 'input');
expect(inputElm.attr('placeholder')).toBe('Test');
expect(inputElm).toBePristine();

browserTrigger(inputElm, 'focusin');
browserTrigger(inputElm, 'focus');
browserTrigger(inputElm, 'input');
expect(inputElm.attr('placeholder')).toBe('Test');
expect(inputElm).toBePristine();

changeInputValueTo('foo');
expect(inputElm).toBeDirty();
}
}));

it('should not dirty the model on an input event in response to a blur', inject(function($sniffer) {
if (msie) {
compileInput('<input type="text" placeholder="Test" ng-model="unsetValue" name="name" />');
browserTrigger(inputElm, 'input');
expect(inputElm.attr('placeholder')).toBe('Test');
expect(inputElm).toBePristine();

browserTrigger(inputElm, 'focusin');
browserTrigger(inputElm, 'focus');
browserTrigger(inputElm, 'input');
expect(inputElm).toBePristine();

browserTrigger(inputElm, 'focusout');
browserTrigger(inputElm, 'input');
browserTrigger(inputElm, 'blur');
expect(inputElm).toBePristine();

changeInputValueTo('foo');
expect(inputElm).toBeDirty();
}
}));

it('should dirty the model on an input event if there is a placeholder and value', inject(function($rootScope) {
if (msie) {
$rootScope.name = 'foo';
compileInput('<input type="text" placeholder="Test" ng-model="name" value="init" name="name" />');
expect(inputElm.val()).toBe($rootScope.name);
expect(inputElm).toBePristine();

changeInputValueTo('bar');
expect(inputElm).toBeDirty();
}
}));

it('should dirty the model on an input event if there is a placeholder and value after focusing', inject(function($rootScope) {
if (msie) {
$rootScope.name = 'foo';
compileInput('<input type="text" placeholder="Test" ng-model="name" value="init" name="name" />');
expect(inputElm.val()).toBe($rootScope.name);
expect(inputElm).toBePristine();

browserTrigger(inputElm, 'focusin');
browserTrigger(inputElm, 'focus');
changeInputValueTo('bar');
expect(inputElm).toBeDirty();
}
}));

it('should dirty the model on an input event if there is a placeholder and value after bluring', inject(function($rootScope) {
if (msie) {
$rootScope.name = 'foo';
compileInput('<input type="text" placeholder="Test" ng-model="name" value="init" name="name" />');
expect(inputElm.val()).toBe($rootScope.name);
expect(inputElm).toBePristine();

browserTrigger(inputElm, 'focusin');
browserTrigger(inputElm, 'focus');
expect(inputElm).toBePristine();

browserTrigger(inputElm, 'focusout');
browserTrigger(inputElm, 'blur');
changeInputValueTo('bar');
expect(inputElm).toBeDirty();
}
}));
});


it('should interpolate input names', function() {
19 changes: 19 additions & 0 deletions test/ng/directive/ngEventDirsSpec.js
Original file line number Diff line number Diff line change
@@ -90,6 +90,25 @@ describe('event directives', function() {

});

describe('security', function() {
it('should allow access to the $event object', inject(function($rootScope, $compile) {
var scope = $rootScope.$new();
element = $compile('<button ng-click="e = $event">BTN</button>')(scope);
element.triggerHandler('click');
expect(scope.e.target).toBe(element[0]);
}));

it('should block access to DOM nodes (e.g. exposed via $event)', inject(function($rootScope, $compile) {
var scope = $rootScope.$new();
element = $compile('<button ng-click="e = $event.target">BTN</button>')(scope);
expect(function() {
element.triggerHandler('click');
}).toThrowMinErr(
'$parse', 'isecdom', 'Referencing DOM nodes in Angular expressions is disallowed! ' +
'Expression: e = $event.target');
}));
});

describe('blur', function() {

describe('call the listener asynchronously during $apply', function() {
24 changes: 21 additions & 3 deletions test/ng/parseSpec.js
Original file line number Diff line number Diff line change
@@ -3,9 +3,11 @@
describe('parser', function() {

beforeEach(function() {
/* global getterFnCache: true */
// clear cache
getterFnCache = createMap();
/* global getterFnCacheDefault: true */
/* global getterFnCacheExpensive: true */
// clear caches
getterFnCacheDefault = createMap();
getterFnCacheExpensive = createMap();
});


@@ -783,6 +785,22 @@ describe('parser', function() {
'Expression: foo["bar"]');

});

describe('expensiveChecks', function() {
it('should block access to window object even when aliased', inject(function($parse, $window) {
scope.foo = {w: $window};
// This isn't blocked for performance.
expect(scope.$eval($parse('foo.w'))).toBe($window);
// Event handlers use the more expensive path for better protection since they expose
// the $event object on the scope.
expect(function() {
scope.$eval($parse('foo.w', null, true));
}).toThrowMinErr(
'$parse', 'isecwindow', 'Referencing the Window in Angular expressions is disallowed! ' +
'Expression: foo.w');

}));
});
});

describe('Function prototype functions', function() {
3 changes: 2 additions & 1 deletion test/ng/snifferSpec.js
Original file line number Diff line number Diff line change
@@ -64,9 +64,10 @@ describe('$sniffer', function() {

it('should claim that IE9 doesn\'t have support for "oninput"', function() {
// IE9 implementation is fubared, so it's better to pretend that it doesn't have the support
// IE10+ implementation is fubared when mixed with placeholders
mockDivElement = {oninput: noop};

expect($sniffer.hasEvent('input')).toBe((msie == 9) ? false : true);
expect($sniffer.hasEvent('input')).toBe(!(msie && msie <= 11));
});
});