Internet Explorer and AutoComplete #122

mbest · 2012-02-26T10:51:25Z

This may be related to case #102.
When using Internet Explorer (version 9.0.8112.16421 in my case) i have a problem with observables not being updated on input change when using the browser's autocomplete.
Here is a recipe for reproducing it.

First create the following page:

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
    <title></title>
    <script type="text/javascript" src="jquery-1.6.1.js"></script>
    <script type="text/javascript" src="knockout-1.2.1.debug.js"></script>
    <script type="text/javascript">
      $(function () {
        var viewModel = {
          firstName: ko.observable()
        };
        ko.applyBindings(viewModel);
      });
    </script>
</head>
<body>
  <form method="get" action="">
    <input name="firstName" data-bind="value: firstName" />
    <span data-bind="text: firstName">INIT</span>
    <input type="submit" />
  </form>
</body>
</html>

Load it in IE (and remember to enable scripts).
Type something in the input field.
Click outside the field and the span to the right will update with your text.
Click the "Submit Query" button. You will remain at the same page but the text you typed should now be stored in the browser's autocomplete cache.
Click on the input field again and click down arrow twice on the keyboard. Your autocomplete text should now be highlighted.
Click on the autocomplete item with your mouse (or simply click tab to move focus away from the input field).
Click outside the input field. Notice that the span to the right is not updated.

See also this page:
http://msdn.microsoft.com/en-us/library/ms533032(v=vs.85).aspx

"To determine when a user updates the content of a field from the AutoComplete dialog box, use the onpropertychange event, rather than the onchange event, because the onchange event does not fire."

The project I'm working on is also using jquery.validate.js. In this framework the validation is triggered correctly also on AutoComplete - so maybe it could be an idea to study how they've implemented it there.

SteveSanderson · 2011-10-12T09:20:47Z

Thanks for reporting this. Sounds like it should be reasonably straightforwards to fix. I'll schedule it for 1.3.1, unless you want to submit a pull request.

thelinuxlich · 2011-11-10T19:01:12Z

+1 on this, it is very annoying!

thelinuxlich · 2011-11-10T20:05:00Z

If you add "onpropertychange" and "DOMAttrModified" events to the list of events watched by the value binding(src/binding/defaultBindings.js#119), it works in IE, FF and Opera.

tehsenaus · 2012-01-30T10:45:40Z

Hi Steve,

I just wanted to check if this fix is applied in 2.0 - the login process on my site relies on Knockout, so it's a pretty serious bug for me!

I'm using a custom version of 1.2 at the moment, so I applied @thelinuxlich 's suggestion and it worked a treat.

SteveSanderson · 2012-01-30T23:48:10Z

We'll have a fix in 2.1.0 if at all possible (that's the same as 1.3.1 in the new numbering scheme).

About propertychange - are you sure this works in IE9? I just tried it, and the event never seems to fire. What did you do to make it work there?

SteffenSH · 2012-01-31T08:11:42Z

My workaround to this problem was to simply disable auto complete in the browser and avoid this situation altogether:
<input .... autocomplete="off" />

I didn't attempt to implement a fix for it.
My best suggestion is the onpropertychange event discussed in the MSDN article.
But if that don't work out I know that jquery.validate.js handles this correctly so it might be an idea to take a look at how they implemented it there.

tehsenaus · 2012-01-31T15:26:04Z

As far as I can remember I couldn't reproduce the bug in IE9, only IE8 (I didn't test earlier versions). Adding "onpropertychange" fixed the problem for me.

domenic · 2012-02-24T01:45:35Z

See also the input event.

Optimize write calls from binding handlers to use a common function. (Makes this change size neutral.)

…ng-term understanding of this. Core mechanism not affected.

SteveSanderson · 2012-02-26T13:59:42Z

This looks great - thanks very much. And good work on identifying the repeated writeValueToProperty logic that could be factored out into a common utility method.

I made slight tweaks only to the code style to help myself be sure I understand what it does (e.g., didn't think that ieAutoCompleteHackNeeded benefited by being a function rather than just a variable). Hope you don't mind. Let me know if you think I've made it worse in any way!

It's a pity we can't have an automated test to verify this works (as it relies on the user clicking on the browser's native autocomplete UI), and I'm not in a position to test it robustly right now. If you're confident that this works, I'll trust you :) -please go ahead and merge it in if so.

mbest · 2012-02-26T22:25:07Z

I made ieAutoCompleteHackNeeded a function because I was thinking of putting it somewhere else, but it works as a variable where it is.

I noticed you changed where I replaced valueUpdateHandler with a new function. I did that so that there would only be one update to the model value. Otherwise both blur and change will update the model, which is different from the existing functionality. What do you think?

kamranayub · 2012-02-29T18:28:28Z

Will this cause KO observables to change on every keystroke now in IE? propertychange fires after every keystroke, rather than when you're "done" editing the input (like the change event).

If so, this would cause issues if you had AJAX calls that depended on an observable (computed) where you'd expect it to fire once when the user is done changing an observable.

mbest · 2012-02-29T19:35:12Z

Will this cause KO observables to change on every keystroke now in IE?

The change to the observable will happen on the blur event.

SteveSanderson · 2012-03-07T10:59:43Z

I noticed you changed where I replaced valueUpdateHandler with a new function. I did that so that there would only be one update to the model value. Otherwise both blur and change will update the model, which is different from the existing functionality. What do you think?

Ah, I see, very subtle!

Thinking about this further, in your original commit 3e63a39, wouldn't overwriting valueUpdateHandler lead to a different breaking change? If the developer tries to respond to multiple events by using valueUpdate: [ someArrayOfEventNames ], then Chrome/Firefox/Safari/etc would fire all of the event handlers, but IE would no longer do that: it would now only fire the first event, after which propertyChanged becomes false and event handlers will be suppressed until the next propertychange occurs.

So, what's the best way to avoid breaking this? One possibility would be we don't overwrite valueUpdateHandler, and for IE in the ieAutoCompleteHackNeeded case, we simply remove change from the array of eventsToCatch. Then IE will still fire all the registered event handlers, except that change is detected by propertychange+blur rather than a real change event.

What do you think? Am I missing any other subtle cases?

mbest · 2012-03-07T20:17:40Z

Steve, You are right that my version would cause a different breaking change. I hadn't thought of that. I suggest we keep it like it is now. Although there will be an additional update in IE, I think that's better than possibly losing updates.

…previously, IE would update the observable twice)

SteveSanderson · 2012-03-09T11:34:31Z

OK then, this all seems to make sense now.

How about we just use your checkIfDifferent flag in the valueUpdateHandler? That way all the browsers will only write to a bound observable once per actual change, regardless of which combination of events triggered it. This is a lot more obvious in my view.

For the majority of cases, it won't actually make any difference, because observables automatically suppress repeated writes of the same value anyway. Developers will only be able to see a difference if they are using the .extend({ notify: 'always' }) option or are using some kind of custom observable.

If you don't see any problems with this approach, please either merge this in, or let me know and I'll do the merge. Thanks!

Internet Explorer and AutoComplete

kyeotic · 2013-10-21T21:36:39Z

Am I missing something? This bug is still showing up for me in IE10 while running in IE9 comptability (or lower): http://jsfiddle.net/KLCqg/2/

kyeotic · 2013-10-21T22:23:41Z

To be clear, the issue I am having is with saved password, not autocomplete on a single box. When you enter a saved username, the password that it put into the password box doesn't get sent to the observable. Even when I tab out of the password box, it remains empty. I have to completely remove the password and enter new one before it gets updated.

kyeotic · 2013-10-22T00:02:15Z

Ok, so more details. First, new fiddle: http://jsfiddle.net/KLCqg/3/ I was using 'onblur', not 'blur' for the valueUpdate. This should be in the documentation.

Second, the fix requires blur to happen on the field that is updated, but in the case of a login form (pretty standard), the username being filled in causes the password to be updated. If users don't move to the password field, but immediately click on the sign in button, password never gets a blur, and the bound observable doesn't get updated. You can see this happen in the fiddle.

I can hack together something that raises blur on the password field, but this is still an issue in the general case.

mbest · 2013-10-22T01:31:37Z

@tyrsius, I seems the problem you are having is unrelated to what we tried to fix with this issue.

I looked into something recently based on a comment on StackOverflow and confirmed that IE10 in compatibility mode has a bug with the autocomplete property.

kyeotic · 2013-10-22T02:27:17Z

Well, I get the same error when running in IE10 without compatibility. I haven't been able to test in a real IE8 or 9, but in IE10 running under any browser mode I get this same behavior: the observable isn't updated.

I may be confused, but that StackOverflow question seems to be discussing a double update. I am not getting any updates. I would be fine with two.

If this is unrelated, then there is a new issue that exhibits the same behavior. Do you want me to open a new issue?

crissdev · 2013-10-22T08:37:49Z

@tyrsius While it may seem you can fix this issue with all sort of events available, I can only say it's painless and useless...For example, you might also have issues with extensions such as Lastpass or Keypass - or even with FireFox's autofill. I ended up getting the values upon clicking the Login button, and put them manually in the observables. It kind of defeats the KO purpose...but I no longer have to fix stupid bugs.
But if you really want to FIX this, then, I would suggest you to look over Angular's implementation.

I found the below script useful for other cases, ie. drag-drop, other IE issues.

function fixInputChangeEvent() {
    var sendChangeNotification = function() {
        if (document.activeElement !== this) {
            // Send change notification only if the active element is different
            jQuery(this).trigger('change');
        }
    };
    jQuery(document.body).on('input', 'input[type="text"],input[type="password"]', function() {
        // Send the change notification
        window.setTimeout(jQuery.proxy(sendChangeNotification, this), 0);
    });
}

kyeotic · 2013-10-22T17:06:29Z

@crissdev I think that would result in a lot more activity than should be necessary to fix the issue. You are setting up a handler for the entire body.

mbest · 2013-10-22T18:22:25Z

If this is unrelated, then there is a new issue that exhibits the same behavior. Do you want me to open a new issue?

Sounds good.

kyeotic · 2013-10-22T18:48:31Z

@mbest I opened #1167

kyeotic · 2013-10-22T20:05:33Z

@crissdev That solution does not appear to work for this case.

mbest mentioned this pull request Feb 25, 2012

2.1 release discussion #338

Closed

mbest and others added 2 commits February 26, 2012 00:50

#122 - update value model on autocomplete selection in IE

3e63a39

Optimize write calls from binding handlers to use a common function. (Makes this change size neutral.)

Added comments and slight tweak of code style purely to aid my own lo…

26c9303

…ng-term understanding of this. Core mechanism not affected.

Continuation of #122 - now ensure observables are only written once (…

e5e7dbd

…previously, IE would update the observable twice)

mbest added a commit that referenced this pull request Mar 9, 2012

Merge pull request #122 from SteveSanderson/122-IE-autocomplete

da28f76

Internet Explorer and AutoComplete

mbest merged commit da28f76 into master Mar 9, 2012

andrewchch mentioned this pull request Mar 26, 2012

propertychange event still not firing in 2.1.0beta #406

Closed

kyeotic mentioned this pull request Oct 22, 2013

IE Autocomplete doesn't update bound observable #1167

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Internet Explorer and AutoComplete #122

Internet Explorer and AutoComplete #122

mbest commented Feb 26, 2012

SteveSanderson commented Oct 12, 2011

thelinuxlich commented Nov 10, 2011

thelinuxlich commented Nov 10, 2011

tehsenaus commented Jan 30, 2012

SteveSanderson commented Jan 30, 2012

SteffenSH commented Jan 31, 2012

tehsenaus commented Jan 31, 2012

domenic commented Feb 24, 2012

SteveSanderson commented Feb 26, 2012

mbest commented Feb 26, 2012

kamranayub commented Feb 29, 2012

mbest commented Feb 29, 2012

SteveSanderson commented Mar 7, 2012

mbest commented Mar 7, 2012

SteveSanderson commented Mar 9, 2012

kyeotic commented Oct 21, 2013

kyeotic commented Oct 21, 2013

kyeotic commented Oct 22, 2013

mbest commented Oct 22, 2013

kyeotic commented Oct 22, 2013

crissdev commented Oct 22, 2013

kyeotic commented Oct 22, 2013

mbest commented Oct 22, 2013

kyeotic commented Oct 22, 2013

kyeotic commented Oct 22, 2013

Internet Explorer and AutoComplete #122

Internet Explorer and AutoComplete #122

Conversation

mbest commented Feb 26, 2012

SteveSanderson commented Oct 12, 2011

thelinuxlich commented Nov 10, 2011

thelinuxlich commented Nov 10, 2011

tehsenaus commented Jan 30, 2012

SteveSanderson commented Jan 30, 2012

SteffenSH commented Jan 31, 2012

tehsenaus commented Jan 31, 2012

domenic commented Feb 24, 2012

SteveSanderson commented Feb 26, 2012

mbest commented Feb 26, 2012

kamranayub commented Feb 29, 2012

mbest commented Feb 29, 2012

SteveSanderson commented Mar 7, 2012

mbest commented Mar 7, 2012

SteveSanderson commented Mar 9, 2012

kyeotic commented Oct 21, 2013

kyeotic commented Oct 21, 2013

kyeotic commented Oct 22, 2013

mbest commented Oct 22, 2013

kyeotic commented Oct 22, 2013

crissdev commented Oct 22, 2013

kyeotic commented Oct 22, 2013

mbest commented Oct 22, 2013

kyeotic commented Oct 22, 2013

kyeotic commented Oct 22, 2013