Form model doesn't update on autocomplete #1460

Closed
msgilligan opened this Issue Oct 15, 2012 · 198 comments

Comments

Projects
None yet
Contributor

msgilligan commented Oct 15, 2012

I'm having problems with Safari 6.0 auto-complete on some simple forms
on Angular 1.0.2.

When Safari uses auto-complete to fill in the form, Angular seems to be
unaware of the new characters that were entered by auto-complete.
Please see the attached image for an example.

To reproduce:

  1. Enter data into the form a few times so that auto-complete is activated.
  2. Then enter a few characters that triggers an auto-complete.
  3. Hit return.
  4. Compare the alert message with the contents of the input field.

I've created a jsFiddle that can be used to reproduce the problem:
http://jsfiddle.net/msgilligan/5cynT/

I've seen a similar problem when Safari 6.0 or Chrome 22.0.1229.79
automatically fills in a password field that has a 'required'
attribute. The form is not marked as valid and I have to go enter a
space after the password and then delete it.

@msgilligan msgilligan closed this Oct 15, 2012

@msgilligan msgilligan reopened this Oct 15, 2012

bigbag commented Oct 17, 2012

A similar problem and Firefox 15.0.1

Contributor

sudhirj commented Nov 1, 2012

How would you handle this? At glance it looks like the browser isn't firing change events if the form is being populated via autofill. The first thing I can think of is to run periodic checks on the form, but that sounds very suboptimal.

Has anymore thought gone into this issue?

pgte commented Dec 12, 2012

Any insights on this one?

bigbag commented Dec 12, 2012

Just banned completion. But this does not solve the problem

+1 for effort towards some sort of solution.

+1. Pls fix this

mgoku commented Jan 1, 2013

I switch from Knockout to Angular just to find that it has the same issue with form autocompletion.

+1 fix please. :(

+1 Please fix!!!!!

sophlee commented Feb 19, 2013

+1 Please fix it :(

lucassp commented Feb 19, 2013

Any progress? +1

The underlying problem I think is that browsers don't (always? ever?) emit any sort of event on auto-complete, as Witold said in this thread: https://groups.google.com/forum/?fromgroups=#!topic/angular/6NlucSskQjY. Other than periodically checking the value in every field that might auto-complete, I bet this is on browser makers to fix, not Angular.

Dave, I understand what you're saying, but Angular still needs to come up with an integrated solution. The reason is that form validation coupled with disabling submit buttons renders the page unusable without special controllers listening for user actions. One solution could be to re-validate the forms automatically after page load, but unfortunately, I haven't found a way to re-fire the validation that way.

I have the same issue on Chrome 25 on Win7. I found it from this issue: #1072 which also provides a temp fix.

+1 Temp fix doesnt work for more complex cases (e.g. a select contains a object as model).

+1... This is a very annoying issue. (Login forms should autofill. Now I can't let them be autofilled.)

augustl commented Mar 11, 2013

I would say the underlying issue is that browsers assumes the form will be submitted in the traditional manner, i.e. the field values will be read upon form submit.

The best fix I can think of is to have a method in AngularJS for reading out ng-model bound values when the form is submitted. Perhaps this could be a generic feature of scopes, such as $scope.$read(), which would be implemented so that ng-model on forms would actually read the .value when this method was called.

augustl commented Mar 11, 2013

Here's an example of a very (!) simple and small ng-model implementation that allows for getting the values on demand. Simply $scope.$broadcast("$myNgModelSet") and the model will be updated.

App.directive("myNgModel", ["$parse", function ($parse) {
  return {
    restrict: "A",
    link: function ($scope, $element, $attrs) {
      // Assume $element is input type="text"
      var modelGet = $parse($attrs["ngModel"]);
      var modelSet = modelGet.assign;

      function setValueFromInputElement() {
        modelSet($scope, $element.val());
      }

      $scope.$watch($attrs["ngModel"], function () {
        $element.val(modelGet($scope));
      });

      $element.bind("change", function () {
        setValueFromInputElement();
      });

      $scope.$on("$myNgModelSet", function () {
        setValueFromInputElement();
      })
    }
  }
}]);

Chrome at least does indeed emit an event (https://code.google.com/p/chromium/issues/detail?id=135307), so I think there is a legitimate issue here.

In addition to a fix for the issue, it would be great to have a sanctioned workaround for browsers that do not emit an event which is explicitly forwards-compatible.

Like a timeout of some sort specifically for forms. Or at least a way to tell angular to re-check.

+1 Please fix this.

I am using a temp hack which is quite dirty:

    $('#my-form input:visible, #my-form select:visible').each(function(){
        $(this).trigger('input');
    });

However, this will trigger another problem. I always got the following errors in concole:

    Error: $apply already in progress
    at Error (<anonymous>)
    at beginPhase (http://ajax.googleapis.com/ajax/libs/angularjs/1.0.5/angular.js:8270:15)
    at Object.Scope.$apply (http://ajax.googleapis.com/ajax/libs/angularjs/1.0.5/angular.js:8072:11)
    at HTMLInputElement.listener (http://ajax.googleapis.com/ajax/libs/angularjs/1.0.5/angular.js:11421:13)
    at HTMLInputElement.jQuery.event.dispatch (http://ajax.googleapis.com/ajax/libs/jquery/1.8/jquery.js:3058:9)
    at HTMLInputElement.elemData.handle.eventHandle (http://ajax.googleapis.com/ajax/libs/jquery/1.8/jquery.js:2676:28)
    at Object.jQuery.event.trigger (http://ajax.googleapis.com/ajax/libs/jquery/1.8/jquery.js:2941:12)
    at HTMLInputElement.<anonymous> (http://ajax.googleapis.com/ajax/libs/jquery/1.8/jquery.js:3599:17)
    at Function.jQuery.extend.each (http://ajax.googleapis.com/ajax/libs/jquery/1.8/jquery.js:611:20)
    at jQuery.fn.jQuery.each (http://ajax.googleapis.com/ajax/libs/jquery/1.8/jquery.js:241:17) angular.js:5687

whuhacker You need to call your hack outside of angular. . Try setTimeout(function() { ... }, 0)
Sorry about the format, doing this with my mobile and SwiftKey.. It's not code intelligent :-)

stephenh commented Apr 1, 2013

@mike-spainhower I believe that bug report is for a different type of auto-fill than a "remember password" type auto-fill.

I've filed a new bug against Chrome here:

https://code.google.com/p/chromium/issues/detail?id=224405

Which links to a JSFiddle that shows the "Remember password" auto-fill does not trigger a change event.

@stephenh Touche! I knew that they were separate under settings but did not realize autofill and passwords had separate behavior - great catch.

Is it still fair to say that angularjs users still need a consistent and supported way to get expected behavior across browsers and versions? From what I gather the closest we have to this currently is setting autocomplete attribute to off.

stephenh commented Apr 2, 2013

@mike-spainhower I agree, that "regular autofill" and "password autofill" is different behavior was surprising to me too.

I think @whuhacker has the best hack for Chrome right now.

For Firefox, it is working for us with this change to Angular:

stephenh/angular.js@f6295cc

If anyone wants to use/clean up/submit that patch, please feel free.

I haven't gotten to trying anything other than Firefox/Chrome--Chrome is being so annoying about this that I'm giving up for awhile and will try again later with a fresh set of eyes.

drozzy commented Apr 2, 2013

Angularjs also does not work with LastPass auto-fill.

stephenh commented Apr 2, 2013

@drozzy How does LastPass auto-fill work? It should either a) fill in the values before body.onload runs (in which case my patch to Angular should fix it, like it did Firefox), or b) fill in the values after body.onload and fire a change event (in which case it should just work, so maybe they aren't firing an onchange).

drozzy commented Apr 2, 2013

@stephenh I don't know... But it doesn't work even if I manually click "autofill" AFTER the page has loaded, through the right-click context menu...

stephenh commented Apr 2, 2013

@drozzy Sounds like a bug in LassPass then--they should fire a change event when they do that, and then Angular will pick it up.

We're also encountering this problem with our app. With LastPass and SuperGenPass, the fields are populated, but the scope isn't updated so when the form is submitted the data passed to our API doesn't match what the user entered. In addition .change() and .trigger(input) or scope.$apply do not result in an updated scope. I suspect because the model doesn't think the data changed, but I don't know. Only solution I've found that has worked is to manually set the scope with jQuery val in the submit angular submit handler which completely overrides the data-binding, and doesn't address the actual issue.

+1 Login forms should autofill.

I came up with a better solution than my previous, but that's not saying much and it doesn't solve the main issue and may well have problems I haven't considered. In my submit handler, I update the value of each input with it's current value using $setViewValue:

$( 'input[ng-model], select[ng-model]' ).each( function() {
  angular.element( this ).controller( 'ngModel' ).$setViewValue( $( this ).val() );
});

+1 works with no form-fill plugin

Contributor

caitp commented May 13, 2014

Since browsers are supposed to dispatch "autocomplete" and "autocompleteerror" events now (and some do), it is probably a sensible idea for angular to handle them.

However, the HTML5 spec says autofilled content to not be validated, so we shouldn't run the validation pipeline when autocompleted, I think.

@matsko can we add something like that into the validation pipeline change?

evictor commented May 13, 2014

@lgalfaso it does exactly the same thing in a slightly different way. Instead of using priority to precede ngSubmit (which I admit is probably a cleaner way of doing this), it unbinds submit, then reattaches a new submit handle that A) triggers the appropriate events and B) $apply()s the nbSubmit expression.

gmoulin commented May 26, 2014

A "fix" for this bug using a modified version of the directive approach, since jqLite does not provide trigger function.
Works fine for a login form.

    .directive('autofillSubmit', [function(){
        return {
            restrict: 'A',
            priority: -10,
            link: function( scope, element ){
                element.on('submit', function(){
                    _.forEach(element[0].querySelectorAll('input'), function( field ){
                        field.focus();
                        field.blur();
                    });
                });
            }
        };
    }])

starting off @evictor solution, I made a form directive decorator for a seamless integration and no jQuery usage, here is the gist: https://gist.github.com/stefanotorresi/1de83e989fd780873af6
will add some tests later.

akofman commented Jun 5, 2014

@stefanotorresi Thx for your solution.
Just a small detail, it seems JQlite find method is based on getElementsByTagName which can't select multiple tags as you suggested it in your gist (line 42).

@akofman thanks! the gist is now updated accordingly

@Pistos Pistos referenced this issue in Pistos/angular-sinatra-authentication-example Jun 11, 2014

Open

Login form doesn't work with Chrome form pre-fill #1

rehand added a commit to rehand/my-grocery-list that referenced this issue Jun 18, 2014

swlasse commented Jun 24, 2014

I used the solution by @evictor - works out of the box - thanks.

Can someone explain why angular can't just double check the actual form fields on ng-submit? This seems really easy and should have been implemented a long time ago.

evictor commented Jun 26, 2014

@ryanmc2033 I agree 100%. Will someone please implement and submit PR? :)

Contributor

caitp commented Jun 26, 2014

@ryanmc2033 if you think it's easy, send a pull request =) Tbosch put together https://github.com/tbosch/autofill-event a while ago as a way to solve this, so you should be able to use that.

I thought it was linked in this issue, but I don't see it.

e-oz commented Jun 26, 2014

@ryanmc2033 it's not enough. Some apps want check values on filling, not on submit.

@jamm yes, but validation only becomes a problem when an autofilled form is submitted. Besides, @caitp said earlier "the HTML5 spec says autofilled content to not be validated,". So we should be skipping validation anyways.

xzer commented Jul 1, 2014

It seems that it has not been resolved yet.

There is a msdn link which explains we should use onpropertychange instead of onchange to capture autofill event.

http://msdn.microsoft.com/en-us/library/ms533032%28VS.85%29.aspx

http://msdn.microsoft.com/en-us/library/ms536956(v=vs.85).aspx

Is this helpful?

xzer commented Jul 1, 2014

I add the following row experimentally and it worked on IE 8!

if ($sniffer.hasEvent('propertychange')) {
element.on('propertychange', deferListener);
}

Contributor

caitp commented Jul 1, 2014

@xzer the "propertychange" event is an IE thing, so this wouldn't benefit most users, or most of the supported browsers, and is very different from detecting "autofill"

xzer commented Jul 1, 2014

yes, it is a IE thing, so we need support it at least for IE8, I test it on IE8 and IE11, sorry I have only this two version in my machine, and it shows that IE11 don't have this event, which means "propertychange" is a IE 8 (or other old version) only event, which also means at least we can make IE 8 work correctly.

Contributor

caitp commented Jul 1, 2014

and is very different from detecting "autofill"

It's more like a primitive version of Object.observe, and you'd run into bugs whenever any property other than value on the input changed (I'd also be surprised if changing the value from script context didn't queue up dispatch of another propertychange event, unless it's even hackier than I thought). Plus, this change would still only apply to input elements, not select elements (for example). And even for input elements, it would not even remotely benefit the vast majority of users.

@tbosch's library is a better solution for this, imo.

xzer commented Jul 1, 2014

you'd run into bugs whenever any property other than value on the input changed
this change would still only apply to input elements, not select elements (for example)

for this event, we can add a wrapper function to fire the original deferListener for input element and value property only.

it would not even remotely benefit the vast majority of users.

since this event is IE 8 only, which means at least IE8 users would benefit from this event and without any side effect for others, imo

and is very different from detecting "autofill"

at the msdn page I pasted, there is a statement which declared that "onpropertychange" is the official way to detect autofill: 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. so I don't think using this event is evil.

+1

@xzer Angularjs is a general-purpose library used by the whole world. Whole world meaning X browsers and their Y versions. Adding code which is known to only be for IE8 is not a very good idea in this regard, is it? Anyways, Angular 1.3 dropped support for IE8 (rightly so), and thus as far as I know, no steps would be taken in this regard from the Angular team.

As a workaround, you can monkeypatch your own angularjs library to include support for IE8 if you wish to do so.

xzer commented Jul 7, 2014

@kumarharsh I agree with your opinion that angular js is designed for all browsers and all versions. I also think this bug fix is not necessary for the newest angular js version. But as a matter of fact, the current stable version shown on the office site is 1.2.x which declares supporting IE 7 & 8. For the old ages, IE only codes, Firefox only codes, Chrome only codes, we were suffering from all of those magic things, I understand it, and the current situation is that there are many developer who are still suffering from those things because the damned XP and old IE which are still used by many big companies, I believe the terrible situation would still continue for about 1-2 years.

In conclusion, I believe it is worth fixing this problem in before 1.2.x versions officially, not my own monkeypatch.

@kumarharsh I actually disagree with your sentiment- nobody likes having to support IE ,
but
that doesn't mean that we should make the users of our websites suffer by choosing to not implement a bug fix. When Internet Explorer was the master, we had to support Chrome, Safari and Firefox users; now that Chrome has all but annihilated every other modern browser in the landscape
( except for Firefox :-P )
we still need to support IE8 until XP dies.
You don't win users by forcing them to do something (see: Microsoft),
you win them by showing them a better way (see: Google / Apple). Chromeframe is a HUGE reason why Chrome has taken over.

TLDR:

it's a bug fix that supports a browser that a few million people still use so a lot of us still have to deal with. If it helps some people, hurts no one, and the solution is in your hands- which is what @xzer appears to be asserting - then why stand in the way?

There are a few bug reports filed for Firefox. I've filed one back in 2008 - https://bugzilla.mozilla.org/show_bug.cgi?id=430931, and there's one recent specific to this thread https://bugzilla.mozilla.org/show_bug.cgi?id=950510. Feel free to click "vote" and comment.

@deckar01 deckar01 referenced this issue in stellar/stellar-client Jul 16, 2014

Merged

Allow angular to listen to autofill events #365

rutwick commented Aug 11, 2014

+1 for the issue. The auto-completed fields have blank values in the models. I tried using some of the directives posted by other users here. But they never work.
Here is a simple fix that I used to get around this problem. In your controller, check if the details exist in the model, if not use the raw form field values.
Example, if your model is car, and the field is car.name, check if the value is empty in the controller, and use the raw text input value [$(...).val()] if yes. Then set the value for $scope.car in your controller.

(Pardon my terminology, I'm new to Angular)

swaron commented Aug 21, 2014

my workaround

var myAutofillDirective = [ function() {
    return {
        require: 'ngModel',
        link:function(scope, element, attr, ngModel) {
            var origVal = element.val();
            if(origVal){
                ngModel.$modelValue = ngModel.$modelValue || origVal;
            }
        }
    };
} ];    

I had to modify @evictor's solution a tad, since I am not using jQuery, and thus don't have .trigger. Also, I only have <input> in my form, and the find('input, select, textarea') did not return anything for me, so I only search for input.

angular.directive('autofillFix', function () {

    return {
      link: function(scope, elem, attrs) {
        // Fixes Chrome bug: https://groups.google.com/forum/#!topic/angular/6NlucSskQjY
        elem.prop('method', 'POST');

        // Fix autofill issues where Angular doesn't know about autofilled inputs
        if(attrs.ngSubmit) {
          setTimeout(function() {
            elem.unbind('submit').bind('submit', function (e) {
              e.preventDefault();
              var arr = elem.find('input');
              if (arr.length > 0) {
                arr.triggerHandler('input').triggerHandler('change').triggerHandler('keydown');
                scope.$apply(attrs.ngSubmit);
              }
            });
          }, 0);
        }
      }
    };
  });

Grrrr, none of the above suggestions worked for me. AngularJS really should not make it more complicated for us. AngularJS shouldn't obstruct the autocompleter of a browser.

e-oz commented Aug 29, 2014

@binarykitchen my dirty hack (#1460 (comment)) still works fine :)
And it's not AngularJS' problem - browsers just don't send events about autocompletion, so blame browsers. Or fork-fix them and send PR )

dlitz commented Aug 29, 2014

"Browsers just don't send events about autocompletion".
Exactly, so it's ridiculous for AngularJS to assume that they do.

AngularJS is a product of Googe so it does make at least a little bit of sense that they do.

So it's a browser issue then? Is there a ticket for that on the Google Chrome issue tracker?

Contributor

PowerKiKi commented Aug 30, 2014

@binarykitchen may I suggest you at least try to search before writing message that are emailed to 108 people. In this very thread the ticket were already mentioned (CTRL+F magic):

Chrome: https://code.google.com/p/chromium/issues/detail?id=135307
Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=430931 https://bugzilla.mozilla.org/show_bug.cgi?id=950510

Also @tbosch may I suggest you lock this thread ? I feel that everything that could be said was already said, with numerous workarounds mentioned. Most notably https://github.com/tbosch/autofill-event which is something that should work for every cases. The discussion is no longer constructive IMHO.

Member

tbosch commented Sep 2, 2014

I agree with @PowerKiKi, locking this thread now.

@tbosch tbosch locked and limited conversation to collaborators Sep 2, 2014

manuroe added a commit to matrix-org/synapse that referenced this issue Sep 11, 2014

Owner

IgorMinar commented Dec 3, 2014

Just an update that the issue is fixed in Chrome M40. Maybe even M39.

There are still issues with Safari 7 and 8. In FF34 the forms autofill works well, but password autofill is still broken.

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