Skip to content
This repository has been archived by the owner on Apr 15, 2022. It is now read-only.

IE9 fixes #15

Merged
merged 3 commits into from
Jan 26, 2015
Merged

IE9 fixes #15

merged 3 commits into from
Jan 26, 2015

Conversation

shawnbot
Copy link
Contributor

These tweaks ensure that ie8.js works in IE9:

  1. Don't return early if (document.createEvent), since IE9 implements this needs shimming elsewhere.
  2. Don't alias Object locally, as this breaks in browserified builds (specifically, when running tests in zuul).
  3. Bail out of commonTextDescriptor() if the fallback descriptor doesn't exist, which fixes IE9 doesn't like the Comment and Text nodeValue shims #14.

Preferably, shims would only be applied if the implementation was broken, but this would require a pretty significant refactor. IE9 gets a bunch of shims that it doesn't necessarily need, but because they work as expected we're good.

@WebReflection
Copy link
Owner

hey @shawnbot I appreciate the effort but I don't understand why you are trying to fix IE9 in a file called IE8. Why don't you use IE8 conditional comment in your HTML templates so nobody will get hurt by this stuff that should not run under any other browser or IE ?

If there are specific IE9 things that you'd like to normalize and fix, why don't you try that after giving dom4 a chance ? That is the normalizer for everything modern, this is only IE8

Whta I mean is that I don't want to fix IE9 here because I might target IE9 but not IE8, which is most common scenario than targeting both.

Does anything I say makes sense to you?

@shawnbot
Copy link
Contributor Author

Yeah, that makes sense. I would suggest giving 2 in my list above some consideration, though. As it is right now, ie8.js is incompatible with browserify, which makes it impossible to use with zuul. I'll wrap the contents of ie8.js in my own conditional when it's included in aight.js

@shawnbot
Copy link
Contributor Author

For what it's worth, though, I'm using both ie8 and dom4 in aight, and they work great. 👍

@WebReflection
Copy link
Owner

Why browserify has anything to do with it? Again, can you use HTML conditional comments in your HTML tempaltes so browserify won't ever possibly even touch IE8 ?

this shoud not affect browserify at all: https://github.com/WebReflection/ie8#ie8-in-cdn right ?

@shawnbot
Copy link
Contributor Author

Take a look at my pull request in aight. I'm using zuul to run automated tests in IE8 and IE9, and zuul wants me to require('aight') in my tests. I can't control the HTML in zuul's test runner, and I actually don't want to because aight is supposed to work outside of conditional comments. (This is so that users can check the aight object to do IE-specific stuff in their JavaScript. Future tests might even ensure that aight hasn't shimmed any APIs that it shouldn't have.) Aight has always been a collection of shims and shams concatenated into a single script for ease of integration, and I would hate to sacrifice that.

@WebReflection
Copy link
Owner

OK, let's say I really don't care much since I don't use ie8 with non ie8 browsers but I'd like to understand if I can merge this. You patched something and now you rolled back? Should I merge this and you are happy or ... should I wait ?

@WebReflection
Copy link
Owner

Apologies but I cannot test anything in VM right now with my setup so I need to trust you or zuul did for me, thanks

@shawnbot
Copy link
Contributor Author

Yes, I think this is worth merging. All it does is comment out the Object = window.Object alias, which doesn't affect the tests as far as I can tell.

In IE8 the "manual native blur" test fails, but I'm not sure if that's because I'm supposed to blur the input manually. When I do so, I get an untraceable script error: 'c' is undefined but the tests run and pass. I get two unrelated tests failing in Chrome:

image

WebReflection added a commit that referenced this pull request Jan 26, 2015
use global `Object` instead of the one that minifies better
@WebReflection WebReflection merged commit 264a15c into WebReflection:master Jan 26, 2015
@WebReflection
Copy link
Owner

Chrome does not behave standard for those tests but trust me, I don't abslutely care and I will not do anything for anything that is not IE8 here so don't waste your time testing other browsers ;-)

@shawnbot
Copy link
Contributor Author

Great, thanks!

@shawnbot shawnbot deleted the ie9 branch January 26, 2015 23:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IE9 doesn't like the Comment and Text nodeValue shims
2 participants