Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Inconsistent behaviour on IE8 #4029

Closed
purcell opened this issue Sep 17, 2013 · 16 comments
Closed

Inconsistent behaviour on IE8 #4029

purcell opened this issue Sep 17, 2013 · 16 comments

Comments

@purcell
Copy link
Contributor

purcell commented Sep 17, 2013

I'm not sure if this is a supported case, but I can use a directive to merge attributes into a DOM element, so given

<input class="typeahead-input" name="query" placeholder="Search" required>

and

angular.module('myapp').directive('typeaheadInput', ["$timeout", function($timeout) {
  return {
    restrict: "EAC",
    replace: true,
    template: '<input autocomplete="off" ng-model="term" ng-change="query()">',
    link: function(scope, element, attrs, controller) {
       ...
    }
  }
});

I'll get this in the DOM:

<input autocomplete="off" ng-model="term" ng-change="query()" class="typeahead-input" name="query" placeholder="Search" required="required">

This is handy because the directive can "plug in" the ng-model and ng-change directives on my behalf.

However, in IE8 I get an unhelpful error: "This command is not supported.undefined". Sigh.

Am I doing something stupid (most likely), or is this a bug?

(This is all with Angular 1.2.0-rc.2.)

@purcell
Copy link
Contributor Author

purcell commented Sep 17, 2013

A corresponding plunker: http://plnkr.co/edit/Dy7FgW?p=preview

Works in Chrome, for example, but (when downloaded and served by local http) fails in IE 8 with "unknown runtime errorundefined".

@petebacondarwin
Copy link
Contributor

The error is being thrown at this point: https://github.com/angular/angular.js/blob/master/src/jqLite.js#L523
This is because IE is a bit picky about what can set innerHTML.
See this article, for instance: http://www.theogray.com/blog/2009/06/internet-explorer-unknown-runtime-error

@petebacondarwin
Copy link
Contributor

It is difficult to fix this bug in any realistic way. The easiest workaround is only to do this sort of thing on a DIV, since IE8 seems happier about DIVs.

@purcell
Copy link
Contributor Author

purcell commented Sep 19, 2013

I'm not 100% sure that's the problem, because in this example the <input> element is being replaced with another <input>, not filled with a new (possibly illegal) element.

@purcell
Copy link
Contributor Author

purcell commented Sep 19, 2013

P.S. Can I ask how you tracked down the error, because I couldn't seem to get any useful info out of the IE8 Developer Tools, and I was frustrated not to be able to at least get a stack trace?

@purcell
Copy link
Contributor Author

purcell commented Sep 19, 2013

I can confirm that switching the initial element to a div does indeed allow IE to proceed. But I'm still not clear why the innerHTML issue would apply here: I'd imagine that innerHTML would be used to place the new <input> in the enclosing element, which is just a div in either case. Perhaps it's related to the sequence of steps Angular takes to perform the replacement.

@purcell
Copy link
Contributor Author

purcell commented Sep 19, 2013

Finally figured out how to debug these issues.

Indeed, the error is actually due to a different (but related) cause: it happens when attributes are copied from the old element to the new element, in mergeTemplateAttributes. At the point this fails, the code is attempting to set the type attribute to "text text", which I guess upsets IE.

In this specific example, the type attribute was set implicitly by IE, because it's nowhere in the original HTML or the replacement template.

I'd naively hope that a workaround may be possible in this case...

@purcell
Copy link
Contributor Author

purcell commented Sep 19, 2013

A simple workaround would be to avoid splicing identical attribute values together with a space: concatenation with spaces likely makes sense only in the case of class.

purcell added a commit to purcell/angular.js that referenced this issue Sep 19, 2013
purcell added a commit to purcell/angular.js that referenced this issue Sep 19, 2013
@purcell
Copy link
Contributor Author

purcell commented Sep 19, 2013

Here's the simple workaround, with a test:

https://github.com/purcell/angular.js/compare/merge-identical-attributes

And then the more rigorous alternative, which is to explicitly prefer the source HTML attributes over those in the the replacement element unless the attributes are class or style:

https://github.com/purcell/angular.js/compare/give-priority-to-original-attributes

purcell added a commit to purcell/angular.js that referenced this issue Sep 19, 2013
This avoids issues with IE 8, where certain attributes cannot be set
on in-document elements, e.g. 'type' on <input>

See angular#4029
@purcell
Copy link
Contributor Author

purcell commented Sep 19, 2013

In fact even that isn't sufficient: the type attribute may not be set at all on an in-document <input> element inside IE8.

The full minimal workaround, then, is the following:

https://github.com/purcell/angular.js/compare/avoid-setting-unchanged-attrs

@thebigredgeek
Copy link
Contributor

Basically, it sounds as though we need a sufficient work around for IE8. Getting/Setting as classes instead of as actual DOM attributes.

@purcell
Copy link
Contributor Author

purcell commented Sep 27, 2013

@thebigredgeek Not exactly sure what you mean.

The workaround I posted above (ie. f919241 + 6d6554f) is working nicely for me, though: without this, it's not even possible to replace an <input> with another <input> in IE8.

purcell added a commit to purcell/angular.js that referenced this issue Oct 1, 2013
Merging an <input> with an <input> in IE8 will automatically break
because the "type" attribute may not be set automatically on in-document
elements. This change makes Angular replace only unchanged
attributes. Additionally, source and target attributes are only merged
by concatenation if their values differ, to avoid unexpected results
such as 'type="text text"'.

Closes angular#4029
@guillaume86
Copy link

@purcell "Finally figured out how to debug these issues."
Do you remember how did you manage to find a stacktrace? I have the same error and IE8 dev tools are desperatly unhelpful

@purcell
Copy link
Contributor Author

purcell commented Jul 9, 2014

@guillaume86 IIRC I had to run it under the debugger and set it to break on errors, but I might be remembering completely wrong.

Re. this particular issue, I've long since abandoned Angular, and I believe Angular has dropped IE8 support, so I imagine this can be closed.

@guillaume86
Copy link

@purcell Yes I have to stay on 1.2.x for that reason (last with IE8 support).
Thanks I managed to see where the error was happening doing just what you suggested.

Note to future self or anyone having the problem:
Apparently it's impossible to get url/line number of javascript errors in IE8 if you use try/catch blocks, only way is to let the errors uncatched reach window.onerrror(message, url, lineNb)... so not possible on angular without massive changes.

@lgalfaso
Copy link
Contributor

lgalfaso commented Jan 8, 2015

This issue is specific to IE8 that only has support in the 1.2.x branch, but this branch is open only for regressions and security issues. Closing this

@lgalfaso lgalfaso closed this as completed Jan 8, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants