Bug when merging attributes in directives replace template

[Hacklone]

I've found this bug when using $compile service on a template that contains a directive with the same attributes.

Possible solution could be changing the mergeTemplateAttributes(dst, src)

function mergeTemplateAttributes(dst, src) {
      var srcAttr = src.$attr,
          dstAttr = dst.$attr,
          $element = dst.$$element;

      // reapply the old attributes to the new element
      forEach(dst, function(value, key) {
        if (key.charAt(0) != '$') {
          if (src[key] && value.indexOf(src[key]) === -1) {
            value += (key === 'style' ? ';' : ' ') + src[key];
          }
          dst.$set(key, value, true, srcAttr[key]);
        }
      });
...

The check for values that are already there is missing.

[lgalfaso]

Hi,
It is not clear what the bug you are describing is. From reading the description I understand that you are claiming that if you have a directive called foo with replace: true and a template as <div class="bar"></div> then when doing

$compile('<div class="bar" foo></div>')($rootScope);

then the result is

<div class="bar bar" foo></div>

and that you think this is a bug. If my understanding is correct, then I can tell you that this is not a bug and works as expected. If the issue is something else, please create a plunker that shows this issue

Thanks

[Hacklone]

I'll create a plunker 4 u :)

[Hacklone]

http://plnkr.co/Zp9TZIvapjTw8J0Xv7ta
So with ng-class and other alike attributres this is quite invalid.

[lgalfaso]

@Hacklone I would agree that concatenating the attribute value of two ng-class can cause issues, but that is another problem that the proposed change does not fix as you can have ng-class="{foo: foo}" on one side and ng-class="{bar: bar}" on the other. To make things a little more complicated, if you have a directive abc with replace: true putting a directive def on the root of abc template and having the same directive def on the element where abc is used causes many issues (and to some extend one of the reasons why replace: true should be avoided).

I am tempted to close this issue as it is a known bug, but will keep it open in case people want to comment on it

[Hacklone]

Then why not just $eval it or I think there's a service like that and extend the original ng-class object with the new one?
Yeah what I suggested is absolutely wrong 😊

[Hacklone]

Or not $eval but something that parses that as it's like a JSON.

[lgalfaso]

I would agree that this is an issue for ng-class just like it is an issue in all cases stated at #7402 (comment) This is a known bug and a limitation with directives with replace: true

[Narretz]

This was actually fixed in 1.2.17 and also in one of the 1.3.0-beta.10. 1ab6e90 for cases where the attributes are identical