regression in 1.2.3: ng-bind-html breaks in Safari 7 on OSX 10.9

[urish]

For some reason, ng-bind-html renders the given text twice on Safari 7. This seems to be a regression in ngSanitize 1.2.3.

The following Plunker reproduces the issue:

http://plnkr.co/edit/4tXYKqp3p5dmiMrmIUNI?

It only reproduces on Safari 7.0 (It didn't reproduce on Safari 6.0.5/Chrome latest/Firefox latest/IE9/IE10). Also, if you change the Plunker to use ngSanitize version 1.2.2, the issue is gone and ng-bind-html functions as expected.

Here is the expected output v.s. the actual output from the above Plunker code:

Expected Output

<div ng-bind-html="name" class="ng-binding"><strong>John Due ☺</strong></div>

Actual Output with Safari 7 and ngSanitize 1.2.3

<div ng-bind-html="name" class="ng-binding"><strong>John Due &amp;#9786;John Due ☺</strong></div>

[caitp]

The issue seems to happen in decodeEntities, and it doesn't really make sense.

If you step through line by line, it actually works correctly --- but if you instead break at the end of the routine, it appears that parts[0] is never cleared (as though line 376 never happened), and so the total match + the inner text are all joined together.

I can't really explain that --- browser bug maybe? but it seems like it's probably easy to work around it

[caitp]

Well, I don't think this is a very good patch, but it does solve the issue --- if someone has an alternative solution that works too, I'm in favour of an alternative.

[urish]

How about parts.shift() and move it to be after the if?

[caitp]

Heh, it is truly bizar.

This fails:

  parts[0] = '';
  if (parts[2]) {
    hiddenPre.innerHTML=parts[2].replace(/</g,"&lt;");
    parts[2] = hiddenPre.innerText || hiddenPre.textContent;
  }

but this passes:

  if (parts[2]) {
    hiddenPre.innerHTML=parts[2].replace(/</g,"&lt;");
    parts[2] = hiddenPre.innerText || hiddenPre.textContent;
  }
  parts[0] = '';

I guess that's probably a better solution though, so I'll try that in the plnkr just to double check

---

http://plnkr.co/edit/8NBJCzK8YMIRGNb8cQ2p?p=preview this version works alright

It's a real shame SauceLabs doesn't support Mavericks / Safari 7 yet :(

[urish]

True, at least now we have a working solution for this. Thanks for taking the time to test and fix this!

[jeffwhelpley]

I ran into this today. It is one of those "this doesn't make any sense" bugs. Glad to see a fix is already in place. Thanks, @caitp.

[scottbaggett]

This issue is back in 1.2.10

[caitp]

Do you have a repro for this?

[scottbaggett]

Unfortunately I don't, I just noticed it on our dev site when I upgraded from 1.2.4 to 1.2.10.

[caitp]

If you can isolate the issue and write a small reproduction that would be a big help, do you have time for that @scottbaggett?

[scottbaggett]

Right now I don't, I'm under a super tight deadline. I've got to roll back for now. I'll try to find some time to help though.

[scottbaggett]

I just updated to v1.2.10-build.2178+sha.40dc806 and the issue is no longer appearing for me. I'm going to go ahead and assume it was something on my end but if I see it again I'll note it and try to help debug it.