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

In IE, tabbing through a field sets it as dirty #1206

Closed
symblify opened this issue Jul 27, 2012 · 19 comments
Closed

In IE, tabbing through a field sets it as dirty #1206

symblify opened this issue Jul 27, 2012 · 19 comments

Comments

@symblify
Copy link

When using the tab key to move through form fields, in Internet Explorer it sets the fields dirty.

The following jsFiddle demonstrates:

http://jsfiddle.net/symblify/XL6RX/1/

Click on the input box and press tab, and the text underneath changes from "false" to "true". It works okay in other browsers.

I think the keyCode for tab needs added to this line:

https://github.com/angular/angular.js/blob/master/src/ng/directive/input.js#L394

@davispw
Copy link

davispw commented Feb 20, 2013

I think the problem is actually on https://github.com/angular/angular.js/blob/master/src/ng/directive/input.js#L402 where it compares new vs. old values. It is comparing null vs. ''. It should normalize null to ''.

@symblify
Copy link
Author

@davispw I disagree. That line is within the event handler and is run by all browsers, so if that was the problem it would affect all browsers. Pressing the tab key should not even trigger the listener function that line is within.

The line I referred to has since moved because of subsequent updates, it is the following line:

https://github.com/angular/angular.js/blob/master/src/ng/directive/input.js#L421

@jbdutton
Copy link

Is this issue resolved now? Can it be closed?

@pkozlowski-opensource
Copy link
Member

@jbdutton I don't think it was fixed yet. But it is a low-hanging fruit so if anyone is up to preparing a pull request it would be surely accepted!

@jbdutton
Copy link

I'll try to give it a shot this weekend.

@symblify
Copy link
Author

Thanks for the PR @jbdutton but as you say in your comment the problem is that there are a lot of keys that are still going to trigger the field being made dirty and it would be a lengthy bit of code to check for all of them. That's part of the reason why I myself hadn't submitted a PR, the other being at the time I logged the issue it was difficult to build Angular on Windows.

I wondered if a solution would be to add the keypress event, which is triggered only when a "printable" character is typed. This would not trigger when backspace or delete is pressed, so keydown would be used to check for them and trigger the listener function. In other words we are turning the current logic on its head and keydown is checking for keycodes which do change the input. Don't know if there are any other "special" keys which change the input?!? Any thoughts on this?

I also wondered whether the change event is necessary now cut and paste are being checked for.

@jbdutton
Copy link

It seems reasonable... we're basically white listing what keys can affect an input. keypress should catch the majority, and then we capture the remainder in keydown. Should we write tests to accompany all of this?

And you're right, I don't see any point to the change event anymore.

@montgomery1944
Copy link

And you're right, I don't see any point to the change event anymore.

Change event is needed for browsers that do not support cut and paste events, for example Opera Classic (http://help.dottoro.com/ljuimtmq.php, http://help.dottoro.com/ljfhhecm.php).

I found also a bug in $sniffer service that will prevent Angular from detecting input event support in Opera Classic. Currently $sniffer service checks support for input event using this code:

  hasEvent: function(event) {
    // 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;

    if (isUndefined(eventSupport[event])) {
      var divElm = document.createElement('div');
      eventSupport[event] = 'on' + event in divElm;
    }

    return eventSupport[event];
  },

The problem is, that Opera Classic will have oninput property only for input tags, but $sniffer service creates div tag to check for oninput property. Due to this bug, Angular on Opera Classic is not using native input event implementation (http://help.dottoro.com/ljhxklln.php). To sum up, we could remove change event listener if we would fix $sniffer service and we would make sure, that other major browsers, especially on mobile, handle cut and paste events.

I wondered if a solution would be to add the keypress event, which is triggered only when a "printable" character is typed. This would not trigger when backspace or delete is pressed

This is not true, keypress event can be triggered also for other keys, that do not change input value, see table on http://help.dottoro.com/ljlwfxum.php and article on http://unixpapa.com/js/key.html.
There is an partial solution for this problem (http://stackoverflow.com/questions/4179708/how-to-detect-if-the-pressed-key-will-produce-a-character-inside-an-input-text/4180715#4180715), but it's not complete, because it will still trigger event for some special keys for some browsers. To sum up, emulating input event is not as simple as we thought. Maybe we could prepare better solution based on articles cited above.

@symblify
Copy link
Author

Thanks @montgomery1944 for your input (no pun intended).

Am I right in thinking that what we might be looking at is:

  1. Fixing $sniffer so it uses an input element rather than a div to carry out the test so that the input event is used in Opera.
  2. Adding logic to the keypress event so that the Esc key is not picked up as a content-changing key.

The keypress, keydown, cut and paste events would apply only to IE, everything else would use input.

Is that right?

@morrisjn
Copy link

morrisjn commented Jan 2, 2014

Hopefully this hasn't already been addressed, but I found that changing this line:

if ($sniffer.hasEvent('input'))

To:

if (!navigator.userAgent.indexOf("MSIE"))

Or whatever way you want to detect IE browsers, fixed this problem.

@tmundt
Copy link

tmundt commented May 9, 2014

The last post of morrisjn seems to be a solution to the problem.
Is that so? And if so, will there be PR?

@caitp
Copy link
Contributor

caitp commented May 9, 2014

The reason it fixes the problem is that IE sends input events for completely silly reasons. I've actually filed a bug with them about it, since they're still doing it in IE11. Hopefully that will eventually be fixed.

I think it might not be a bad idea to just not listen to the input event for IE at all, though, and it would mean we could throw away my lame hack around the placeholder issue. Have a go at implementing it

@seaside98
Copy link

+1

In IE11, just clicking on the field sets it as dirty.

@caitp
Copy link
Contributor

caitp commented Jun 14, 2014

@seaside98 there's no way for us to fix this in angular...

As a workaround, you can avoid using a placeholder... It's stupid, but that will do it... I filed an issue on MS's feedback thingy --- you can +1 it or whatever, but yeah basically IE is doing the wrong thing, and they've triaged it as a pretty low priority, so you might not see a fix until IE12 or even later.

The issue I've filed is at https://connect.microsoft.com/IE/feedback/details/856700/ie11-ie10-send-input-events-at-times-when-it-is-not-appropriate if you want to add to it

@cvn
Copy link

cvn commented Dec 4, 2014

This issue was fixed by #9265

EDIT: I originally posted a different comment here, one meant for another issue. Sorry if anyone got a confusing email.

@jbedard
Copy link
Contributor

jbedard commented Dec 4, 2014

EDIT: I originally replied to @cvn here :P

@caitp
Copy link
Contributor

caitp commented Dec 4, 2014

@bterlson I really, really want the input event to be fixed at some point :( it would be so great if we could just say "don't care about legacy IE, it works with new IE, go buy windows 8 :<" --- all of these hacks to work around it .______.

@pkozlowski-opensource
Copy link
Member

Indeed, it is fixed in master: http://jsfiddle.net/du6zuuow/ - tested with IE9.
If anyone see this bug with other version of IE please open a separate issue with a reproduce scenario.

@bterlson
Copy link

bterlson commented Dec 6, 2014

@caitp Just FYI, from my experimentation I believe this is fixed in the IETP (with experimental features enabled). Could use RemoteIE to double check :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests