IE8 Support
matthew-andrews opened this issue · 26 comments
In #42 @wheresrhys said:
I'm finding it's not the only place where ftdomdelegate doesn't cut our ie8 mustard (e.g. its use of
Node). To be able to use it we'll need to either
- Introduce some regressive compatibility changes
- Drop ie8 support, as @commuterjoy is very keen to do
- Properly fork it as opposed to wrapping it for use in
Financial-Times/ft-origami#189- Have a compatibility version alongside the main one
Thoughts? @triblondon
My personal feeling is we should try to explore option 1 first.
If it's just the use of the constant on the Node object I think IE8 support should be fairly trivial... http://stackoverflow.com/questions/13377887/javascript-node-undefined-in-ie8-and-under
Seems like we could easily add a 'micro-polyfill'? Or are there other uses of Node that cause problems?
Errors thrown in ie8 so far
- Node.TEXT_NODE (297)
- Failing to retrieve
listenerListfor scroll event (not sure what the exact cause is though)
If you want to hack that to be Node.TEXT_NODE to 3 for now you might have more luck.
@wheresrhys - I've created a branch with this hack in - also added an origami.json - would you mind checking this version?
https://github.com/ftlabs/ftdomdelegate/compare/origami?expand=1
I already put that temporary fix in locally, but the scroll issue was still present. Is your test runner able to generate a html page to run the tests? Or are you able to get it running the tests in ie8 easily some other way? (this would be ie8 with the event listener polyfill present as a helper)
As long as the IE8 VM or machine can see a webserver on your computer, you should be able to run the test suite by following the following steps
- git clone the ftdomdelegate repo to your machine (I'm assuming it's os x / linux)
cd ftdomdelegate && npm installbuster server &(and wait for it to say:buster-server running on http://localhost:1111)- within IE 8 go to
your-machines-ip-address:1111and click capture browser:
- go back to your *nix machine and run
buster test.
Possibly sensible to test that with Chrome first. Also believe you can attach more than one browser at the same time to test both.
(You might need to prefix the buster commands with ./node_modules/.bin/buster)
To get the event listener polyfill helper in there... at a path to it in the sources array within here:-
https://github.com/ftlabs/ftdomdelegate/blob/master/test/buster.js#L7
Getting somewhere now. Buster's now on 0.7 so I needed sudo npm install -g buster@0.6.x. Add to README? Or update the buster config?
- Ideally: replace buster with mocha so that we can do cross-browser testing with testling
- Alternatively: Update readme to change the commands from (eg)
buster serverto./node_modules/.bin/buster serverto force it to use the version of buster installed for the checkout by npm.
Don't really think it's worth updating to the latest version of buster but it's up to you really...
Slightly off topic, but 'Returning false from a callback should preventDefault' fails in IE-edge. Isn't this just native behaviour in browsers where this test passes, and if so arguably should not be part of the test suite. If you agree I'll remove it in this branch.
I'm not too familiar with the way that has been implemented (one of the bits that Matt CG wrote before I took over this library) but I believe we implement that behaviour here:-
https://github.com/ftlabs/ftdomdelegate/blob/master/lib/delegate.js#L344
So as far as I know the test ought to be testing code within this project.
Ah, I missed that. I'll raise a bug. Really strange behaviour. defaultPrevented is being set to true but by the time the timeout has run it's ben reset back to false. Not sure if it's the src or the test that's at fault.
If you comment that test out do we have IE8 support?
We might be able to just remove that feature as I don't think that it's one we use very much... (Although I will need to check)
Not sure - just finished rewriting the tests so they run in ie8, but mostly failing for now.
Could you give me permissions to push to the origami branch please - ta.
Try now?
Ta. Well, now we have a test suite that mostly runs in ie8 but fails for the most part. The polyfill I'm using doesn't define eventPhase which is part of the problem, but even adding a hacky ev.eventPhase = ev.target === ev.currentTarget ? 2 : 3, which I thought would fix a few tests, has little effect.
ie8 is indeed the new ie6!
Urgh..
No support for capture phase.
http://robertnyman.com/2008/11/04/internet-explorer-8-fix-event-handling-or-dont-release-it/
If the eventPhase is not defined try setting it to 2.
Setting it to 3 by default, but 2 if target and currentTarget match fixes 2 more tests by fluke, but still a long way off satisfactory.
Is that 2 more tests compared with always setting it to 2?
As the hooking into the capture phase is quite a fundamental part of this library I'm not sure we can offer full IE8 support :-/
Yep. I've since realised I needed a matchesSelector polyfill and the test score's looking a lot healthier now - 4 failures, 2 errors - but the remaining bugs are most likely insurmountable (at least not without jquery-levels of browser hacks)
Have you seen this:
https://gist.github.com/jonathantneal/2415137
That's the one I'm using. It polyfills pretty well an event on a given element but it doesn't look like it can affect it's propagation much
Oh I see... What do you think we should do?
3 options I suppose
- Forget ie8 in both ftdomdelegate and origami
- Merge the origami branch and document all the ie8 bugs (which are fairly limited, and for the most common use case - bubbling clicks - it works fine)
- fix ie8 in the origami branch but don't merge back in
2 is probably the most realistic, from both development and getting official approval perspectives
I'd be comfortable with merging the origami branch. Do you want to make a pull request and assign to me for a final review?