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

fix(inputs): improve "input" event emulation for legacy browsers #9623

Closed
wants to merge 1 commit into from
Closed

fix(inputs): improve "input" event emulation for legacy browsers #9623

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 15 additions & 4 deletions src/ng/directive/input.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -994,16 +994,27 @@ function baseInputType(scope, element, attr, ctrl, $sniffer, $browser) {
}
};

element.on('keydown', function(event) {
var key = event.keyCode;
element.on('keypress', function(event) {
var key = event.keyCode,
multiline = event.target.nodeName.toLowerCase() === 'textarea';

// ignore
// command modifiers arrows
if (key === 91 || (15 < key && key < 19) || (37 <= key && key <= 40)) return;
// escape enter
if (key === 27 || (key === 13 && !multiline)) return;

deferListener(event);
});

element.on('keydown', function(event) {
var key = event.keyCode;

// only fire listener for a few extra keys not captured by "keypress"
// backspace delete
if (key === 8 || key === 46) {
deferListener(event);
}
});

// if user modifies input value using context menu in IE, we need "paste" and "cut" events to catch it
if ($sniffer.hasEvent('paste')) {
element.on('paste cut', deferListener);
Expand Down
4 changes: 2 additions & 2 deletions src/ng/sniffer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ function $SnifferProvider() {
if (event == 'input' && msie == 9) return false;

if (isUndefined(eventSupport[event])) {
var divElm = document.createElement('div');
eventSupport[event] = 'on' + event in divElm;
var elm = document.createElement('input');
eventSupport[event] = 'on' + event in elm;
Copy link
Contributor

Choose a reason for hiding this comment

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

Was there a case where this change was required?

Copy link
Author

Choose a reason for hiding this comment

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

@jbedard This allows Opera versions with the Presto engine to use the real "input" event instead of the emulation. Opera Desktop and Mobile up through versions 12 and all versions of Opera Mini only have the input event on form elements (inputs, textareas, and selects). I learned about this from a tip by @montgomery1944 in the #1206 thread.

Here's a test result from Opera 12.16 mac that confirms this: http://i.imgur.com/Carhv3n.png. Opera's support for the input event is pretty good, in my testing. The only flaws I found are that it will fire an input event when pressing the "enter" key in a single-line field.

EDIT: I didn't consider divs with the contenteditable attribute until now. These are supported in Presto, but without the input event. See table here. In other words, Opera's support for the input event is incomplete and we shouldn't rely on it. Forget about this change.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think old Presto versions of opera are too old and unsupported anyway...

}

return eventSupport[event];
Expand Down
19 changes: 17 additions & 2 deletions test/ng/directive/inputSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1495,7 +1495,7 @@ describe('input', function() {
}
});

describe('"paste" and "cut" events', function() {
describe('legacy events', function() {
beforeEach(function() {
// Force browser to report a lack of an 'input' event
$sniffer.hasEvent = function(eventName) {
Expand All @@ -1521,8 +1521,23 @@ describe('input', function() {
expect(scope.name).toEqual('john');
});

});
it('should update the model on "keypress" event', function() {
compileInput('<input type="text" ng-model="name" name="alias" ng-change="change()" />');

inputElm.val('cassius');
browserTrigger(inputElm, 'keypress');
$browser.defer.flush();
expect(scope.name).toEqual('cassius');
});

it('should not dirty the model on "keydown" event', function() {
compileInput('<input type="text" ng-model="name" name="alias" ng-change="change()" />');

browserTrigger(inputElm, 'keydown');
$browser.defer.flush();
expect(inputElm).toBePristine();
});
});

it('should update the model and trim the value', function() {
compileInput('<input type="text" ng-model="name" name="alias" ng-change="change()" />');
Expand Down
12 changes: 6 additions & 6 deletions test/ng/snifferSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,34 +25,34 @@ describe('$sniffer', function() {


describe('hasEvent', function() {
var mockDocument, mockDivElement, $sniffer;
var mockDocument, mockElement, $sniffer;

beforeEach(function() {
mockDocument = {createElement: jasmine.createSpy('createElement')};
mockDocument.createElement.andCallFake(function(elm) {
if (elm === 'div') return mockDivElement;
if (elm === 'input') return mockElement;
});

$sniffer = sniffer({}, mockDocument);
});


it('should return true if "onchange" is present in a div element', function() {
mockDivElement = {onchange: noop};
mockElement = {onchange: noop};

expect($sniffer.hasEvent('change')).toBe(true);
});


it('should return false if "oninput" is not present in a div element', function() {
mockDivElement = {};
mockElement = {};

expect($sniffer.hasEvent('input')).toBe(false);
});


it('should only create the element once', function() {
mockDivElement = {};
mockElement = {};

$sniffer.hasEvent('change');
$sniffer.hasEvent('change');
Expand All @@ -64,7 +64,7 @@ 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
mockDivElement = {oninput: noop};
mockElement = {oninput: noop};

expect($sniffer.hasEvent('input')).toBe((msie == 9) ? false : true);
});
Expand Down