ngModel renames inputs with no "name" attribute to "undefined"

[itsananderson]

The following Plunker illustrates the issue (see the console output).
http://plnkr.co/edit/lZEgQv05Dm7YGelXZDhg?p=preview

When an input uses the ng-model directive, the following code watches for the "name" attribute to change.

src/ng/directive/input.js:2456

attr.$observe('name', function(newValue) {
    if (modelCtrl.$name !== newValue) {
        formCtrl.$$renameControl(modelCtrl, newValue);
    }
});

introduced: 729c238
modified: b1ee538

The problem is that this observer fires with undefined if the input doesn't have a "name" attribute, causing the control to be renamed to undefined. If multiple inputs are missing the "name" attribute, they are all renamed to undefined, and overwrite each other in the formCtrl object.

I propose a fix:

attr.$observe('name', function(newValue) {
  if (undefined !== newValue && modelCtrl.$name !== newValue) {
    formCtrl.$$renameControl(modelCtrl, newValue);
  }
});

And a test that fails before the fix and passes after the fix:

// test/ng/directive/inputSpec.js
it('should not rename form controls in form when name is undefined', function() {
  compileInput('<input type="text" ng-model="name" />');
  expect(scope.form['undefined']).toBeUndefined();
});

I'm happy to submit this as a PR, but I wanted to file an issue first to confirm that this is unexpected behavior, and that my fix looks OK.

[rodyhaddad]

+1 for the proposed fix

An input with no name shouldn't end up on the formController object

[Puigcerber]

Hi @rodyhaddad! Here in http://ngeurope.org/

Should we check for the existence of a name before adding the control in formCtrl.$addControl(modelCtrl);?

[Narretz]

@Puigcerber This would be a breaking change: currently, the control is added to the form regardless of name, but it is only published on the form when a name is set:

angular.js/src/ng/directive/form.js

Line 122 in b1ee538

form.$addControl = function(control) {

It's a little weird.

[pkozlowski-opensource]

@Narretz I think this code was introduced when the $setPristinemethod was added. You need to have the controls collection to be able to manipulate them (here: reset them to their pristine state) but adding controls without name to a form controller doesn't make sense since no one could access them.

So yeh, it is not that weird :-)

[rodyhaddad]

I talked with @Puigcerber in person at ngeurope, he's currently investigating why $interpolate is returning a function in attrInterpolatePreLinkFn even though the attribute doesn't exist on the element.

See

angular.js/src/ng/compile.js

Lines 2304 to 2309 in 89c57a8

	interpolateFn = $interpolate(attr[name], true, getTrustedContext(node, name),
	ALL_OR_NOTHING_ATTRS[name] || allOrNothing);
	// if attribute was updated so that there is no interpolation going on we don't want to
	// register any observers
	if (!interpolateFn) return;

[Puigcerber]

Hi @rodyhaddad! I checked it but $interpolate does not return a function if the name is undefined and it breaks properly in L2275.

[Puigcerber]

So what I can think of is not to add a listener if attrs[key] is undefined.
Not to call the function manually if attrs[key] is undefined.

[dlongley]

I left a line note about this, but perhaps should have just made it a comment, so I've copied it here for anyone else who runs into this problem:

With 531a8de, if anyone was using attrs.$observe() with an optional attribute (and perhaps using it to set a default value), the handler will no longer execute, which may break your app. If you need the handler to execute, I recommend setting a default value (attrs.foo = attrs.foo || 'foo') prior to calling $observe.