Fix two IE8 JavaScript bugs

[zarino]

Your README includes a "Support for old IE" section, so I’m assuming IE8 support is something you want to maintain.

I found two IE8 bugs in taggle.js while attempting to reuse it in a mySociety project.

1. It assumed HTMLElement existed – whereas IE8 has no HTMLElement, so you have to use Element in IE8 instead
2. It attempted to change the type property of the "Close" button after the element had been created – whereas IE8 doesn’t let you modify special properties of <input> and <button> elements (like type and name) after they have been created

This PR includes fixes for both bugs.

There’s unfortunately another IE8 bug I spotted—this time with the example CSS—but sadly didn’t have time to fix. For posterity:

1. In IE8, when using the styles from /example/css, the .taggle_input is always invisible. You can focus the input (without getting any visual feedback) and even type into it (without seeing any of the text), and when you hit , or Enter, the tag gets created as you’d expect. I think the issue is that you’re setting .taggle_input to width: 100% but its parent li has no explicit width, so IE8 assumes a width of 0. Setting a min-width of 10% or whatever on .taggle_input fixes the invisibility problem, but reveals a black border around the element, that I can’t work out how to remove.

Since the example styles are only included as an indication of how you could style the component—and most people will be writing their own styles—I guess maybe this invisibile input bug isn’t a deal-breaker. But I figured I should mention it.

[okcoker]

Thanks for this @zarino. Seems good overall but I have a few questions/comments

It assumed HTMLElement existed – whereas IE8 has no HTMLElement, so you have to use Element in IE8 instead

Is this because of the use of .tabIndex?

While you're in IE8 land, could you see if the real solution to the attribute problem is to just use button.setAttribute('type', 'button')?

If that's the case, there are a few other places that do the same attribute modification so wouldn't this problem still exist there? I was wondering why there was only one place where this was needed.

Not sure why the build is crashing but hopefully that's a fluke.

[zarino]

Is this because of the use of .tabIndex?

The error was 'HTMLElement' is undefined and it was raised by this line where you check that tagFormatter returns an li element:

(To get the code into a state where you can try this out, you need to fix the close.type error, and then try adding a taggle tag.)

As far as I can tell, IE8 has no concept of HTMLElement:

> document.createElement('li') instanceof HTMLElement
× 'HTMLElement' is undefined
> document.createElement('li') instanceof Element
true

While you're in IE8 land, could you see if the real solution to the attribute problem is to just use button.setAttribute('type', 'button')?

Genius! Everything I read online suggested there was simply no way of changing the type of a button after it had been created, but I never thought to try setAttribute. Lo and behold, setAttribute("type", "button") appears to work in IE8 – clicking the button does not submit its parent form.

I’ve updated my commit to use setAttribute instead, in all browsers. Tested and works fine in IE8–11, and modern versions of Chrome, Firefox and Safari on desktop and mobile.

If that's the case, there are a few other places that do the same attribute modification so wouldn't this problem still exist there? I was wondering why there was only one place where this was needed.

This puzzled me at the time too. I can confirm that, in IE8:

> var button = document.createElement('button')
undefined
> button.outerHTML
"<BUTTON type=submit></BUTTON>"
> button.type
"submit"
> button.type = 'button'
× Object doesn't support this action
> button.outerHTML
"<BUTTON type=submit></BUTTON>"
> button.type
"submit"

> var input = document.createElement('input')
undefined
> input.outerHTML
"<INPUT>"
> input.type
"text"
> input.type = 'hidden'
"hidden"
> input.outerHTML
"<INPUT type=hidden>"
> input.type
"hidden"

As far as I can tell, we can access the type property of both inputs and buttons, but we can only modify the type property on an input.

Not sure why the build is crashing but hopefully that's a fluke.

Looks like Travis hit a Cloudflare rate limiting error from packages.npmjs.org when trying to npm install.

[coveralls]

Coverage remained the same at 86.19% when pulling 535eb46 on mysociety:master into 065f1b3 on okcoker:master.

[okcoker]

Gotta love IE 👍 Thanks for these fixes @zarino

[zarino]

No problem! Thanks for sharing the library :-) In case you’re interested, here’s the project we’re using it in: mysociety/foi-for-councils#112