Fix Internet Explorer invalid <input> "type" attribute runtime exception

[peteygao]

An attempt at addressing the <= IE11 run time exceptions first discovered in Issue #72.

Terminology

First, a distinction between two similar terms has to be made:

1. "Attribute" refers to an actual attribute in the HTML document:

   e.g. <input type="text"> In this instance, type is the attribute, its value is "text".

2. "Property" is a .property attached to a JS DOM object:

   e.g. imgNode.src = "https://example.com/cat.gif" src is the property on the imgNode object.

Problem

The issue is that IE only allows a specific list of values for its <input> element's type property, namely values which have actual implementations backing them (e.g. radio, text, button, etc). This list increases from version to version, as the IE team implement more input types (e.g. type="range" is not supported in < IE9, but is supported in IE10+).

Unsupported values for type (e.g. type="date") will cause the JS execution context to throw an exception when attempting to assign the type property (e.g. inputElement.type = "date") with the exception Invalid Argument (even though no function is actually called...).

Solution

This patch alleviates this problem by catching the "Invalid argument." error that IE throws and directly setting the DOM level type attribute using setAttribute, which will correctly set the type attribute AND property on IE (invalid values when using setAttribute is safely ignored as per this comment as well as empirical evidence from my own tests).

I'm retaining existing functionality in modern browsers by having domNode.type = value be the default try action, while the catch fallback for IE is set to use setAttribute.

Performance analysis in a follow up comment.

[process-bot]

Thanks for the pull request! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

[peteygao]

Performance analysis and optimization

Attempt 1:

I opened this PR with commit cf77bd5.

Afterwards, I profiled its performance characteristics (https://jsperf.com/setattribute-vs-property-value). The results weren't good:

Apart from the fact that IE9 is no longer ERROR'ing while using setAttribute, it was a performance regression across the board.

So, back to the drawing board we go.

Attempt 2:

Instead of using setAttribute even when domNode.type = value is supported, why not just catch the exception for IE browsers that hit this particular snag? This change is in commit 85f5931.

Let's have the numbers speak for themselves (https://jsperf.com/set-property-value-with-try-catch-setattribute-fallback/9):

Over all the results were largely positive, with FF and Safari showing literally no performance loss, while Chrome shows a 22% decrease in performance just by creating the try... catch context (seems like an optimization that the Chrome/V8 team can make to bring its performance on parity with Safari/FF). IE10 and Edge both show some performance loss, while IE9 actually can complete the test without ERROR when using the code with the fallback.

Note that I'm not trying to catch all exceptions wholesale. The code looks for an IE specific "Invalid arguments." error message, and throws all other exceptions if it does encounter them.

[evancz]

Great work on this! Very thorough evidence and explanation! Very well done!

Based on #10, it appears that type_ can just crash on "invalid" values. So it may be best to just catch all errors and not try to match on a particular string that may not match between IE versions or different browsers.

I'm also uncertain if silent failures are better than noisy ones on this. Before making a choice, I would like to get some usage stories on what people want in practice when they run into this. Is there a clean fallback somehow?

[rtfeldman]

I'm also uncertain if silent failures are better than noisy ones on this.

@evancz You can have both. Catch the error and then do this:

console.warn(error.stack);

This will print the stack trace to the console, formatted as a warning. Now you still have the info, but end users don't suffer a crash. (Could even do better by logging some Elm-specific info, e.g. "you were making a select element and gave it a type_ value of blah")

[peteygao]

@rtfeldman & @evancz Thanks for the feedback guys. I'm happy to add the stacktrace logging via:

console.warn(error.stack);

Need some input with wording the friendly help message since it's technically not the case that the type is necessarily "wrong" when this occurs. It happens to not be supported by a particular IE implementation.

What do you guys think of:

This browser does not support setting the `type` attribute of <element> to "value".

e.g.:

This browser does not support setting the `type` attribute of <input> to "range".

EDIT: I forgot to ask: are Elm warning/error messages prefaced with anything?

[rtfeldman]

That sounds super clear and helpful - love it!

[peteygao]

@rtfeldman Error logging is committed ✨!

[muelli]

what's the status here? Is it worth the effort of forward porting this patch or will it be ignored until the crashing IE version is being EOLed?

[peteygao]

@muelli Thanks to your message, I came back to this PR (holy cow, it's been a year and half since I made it) and checked out the latest state of VirtualDom.js. It looks like this patch is no longer needed as this function now implements setting/removing attributes using setAttribute and removeAttribute, which is essentially how I resolved this issue in the first place. I'm closing this PR now 💯!

[muelli]

oh, now I'm curious. Because I'm running in the frustrating situation that I cannot have my app run on IE11, because Elm does not catch that exception thrown by IE. I'm trying to render input [type_ "date"] [] which makes the app crash on IE. The polyfills I've tried don't make the JavaScript property access work.

Now I'm interested in learning how you worked around that issue.

[evancz]

@muelli, you can do Html.Attributes.attribute "type" "date" to get virtual DOM to use setAttribute in JavaScript. (Similarly you can use Html.Attributes.property to set the property.)

When there is a 0.19 alpha out to try, do you mind testing if this is fixed? If not, please let me know and I can try to figure out a way that does not need if or try or anything. (Could be just swapping the Html.Attributes.type_ to be an attribute, but that may cause other issues!)

[muelli]

you can do Html.Attributes.attribute "type" "date"

that works. yay. And the https://www.npmjs.com/package/date-input-polyfill seems to work, too. yay :)

When there is a 0.19 alpha out to try, do you mind testing if this is fixed?

probably not. What's the best way to be informed about a release? A mailing list doesn't seem to exist. (but this is getting OT now..)

but that may cause other issues!

I appreciate that the problem space is terribly complex. Thanks for keeping things running in such a harsh environment!