[BUGFIX beta] Remove attributes that are set to null or undefined.

[dgeb]

This resolves a problem in the legacy bind-attr implementation in
which attributes would be set to null instead of being removed.
This led to spurious fetches for src attributes of img elements
and may have had other side effects.

[Fixes #10675]

[rwjblue]

This is somewhat related to the conversation we had at #10687 (comment).

@mixonic - r?

[mixonic]

:-/ @dgeb So src is set using a property, since src is a property on document.createElement('img').

Can we add src to a list that already includes value, here: https://github.com/dgeb/ember.js/blob/bug-fix-10675/packages/ember-views/lib/attr_nodes/legacy_bind.js#L32

And include both null and an undefined in the values which are known to not actually be clearing? Instead both src and value expect to see '' for clearing.

At a later time, this should probably move to the dom helper.

[dgeb]

@mixonic I might be misunderstanding you, but if I change:

  if (this.attrName === 'value' && value === null) {
    value = '';
  }

To:

  if ((this.attrName === 'value' || this.attrName === 'src') && value === null) {
    value = '';
  }

Then the src attribute is not cleared by simply setting its value to an empty string. It doesn't seem to be cleared without calling removeAttribute.

Note that value is set to null already if it's undefined.

Please let me know if I'm misunderstanding your suggestion.

[mixonic]

@dgeb it won't make a request for the asset with '' though.

I'd prefer to not lean on removeAttribute when we normal deal with property values for value and src. The inconsistency sits uncomfortably with me, I'd rather set a neutral property value. I'm on IRC if you want to kick it around.

[dgeb]

@mixonic - gotcha - will revise this PR.

[dgeb]

@mixonic @rwjblue revised - please review.

[rwjblue]

LGTM

[rwjblue]

Pulling into beta (need commit prefix next time though)....