issue/142-browser: Amends for IE8-9 browser support

[oliverfoster]

142

Code changes

• Reverted buildMultipartPost to the pre Uint8Array and Blob implementation to provide IE8-IE9 attachment support.
• Removed unused functions ADL.stringToArrayBuffer, ADL.stringFromArrayBuffer (redundant as the UInt8Array using text-encoding code has been reverted).
• AddedObject.keys shim. Ref

Doc and testing changes

• Forced browser examples and tests to run in non-compatibility mode in IE8 and IE9. Ref
• Switched examples and tests to use jquery v1 instead of jquery v2 for IE8 support. Ref
• Fixed bad preventDefault in useWrapper.html stoping option selection in IE8.
• Fixed useWrapper.html to correctly report returned value when using XDomainRequest in IE8.
• Removed mocha blanket test coverage plugin as unsupported in IE8.
• Removed assertion libraries should and expect and replaced with an IE8 compatible assert library.
• Changed all tests to use assert assertion library so they run in IE8.
• Modified mocha.css to provide IE8 support on :before and :after. Ref

Notes on testing

• IE8 and IE9 VMs can be fetched from Microsoft for testing. Ref
• You should run useWrapper.html from an HTTPS server as XDomainRequest is requesting from an HTTPS LRS and the cross domain requests over XDomainRequest must have the same protocol. Ref.
• XAMMP contains a preconfigured HTTPS webserver if you need one for testing. Ref

Conclusion

• All tests working in browser (when on an en-US language code).
• Library works in IE8 and IE9.

[vbhayden]

Was coming to recommend it target a non-master branch, looks like you had the same idea.

Will look through this now.

[oliverfoster]

Is the meeting happening today? I seem to have missed it two weeks in a row.

[vbhayden]

For the polyfills, is there a reason the recommended prototype shims weren't implemented instead?

[oliverfoster]

It's generally bad practise (as far as I'm aware) to leak behaviour into the global namespace when it's both unnecessary to do so and could cause unintended consequences. This library will be used in other systems which may have their own shims or feature detection behaviour which rely on these same global references.

There are some good articles on the subject ref, ref1

[vbhayden]

Those are only set up to define functions on those objects if the intended function doesn't already exist, though.

Since those functions are designed to replicate functionality from later ES_ updates, the only notable problem you might bump into is if the wrapper is accidentally providing functionality to another embedded service that didn't account for the browser's JS objects not having those definitions natively. I think the prototype definitions are probably the cleanest way to go and would reduce the number of changes required to facilitate ie8.

[oliverfoster]

Ok

[oliverfoster]

Shall I shim both the Object.keys and Date.prototype.toISOString behaviour?

[vbhayden]

I think that will be the cleanest way, yeah. The fewer changes required for backwards compatibility the better, as ideally we will be merging master changes into the legacy branch when possible.

[oliverfoster]

I was planning to make substantial changes to master once I'm done preserving the IE8/9 behaviour, keeping the two branches in sync is going to prove quite difficult.

[vbhayden]

Well any substantial changes would require a substantial reason for being made. The one area that could use some love is the testing department, so you're welcome to submit a PR with those sorts of changes.

This wrapper is used across quite a few different projects, so even small changes need to be vetted to ensure that our users maintain functionality etc.

The likelihood of a PR being approved is usually inversely proportional to its size. If we can go in, easily see what's being changed, and quickly make a determination about its value / necessity, then everyone's lives are much easier.

[oliverfoster]

As previously discussed, I was planning on fixing the docs, updating grunt for source-mapping, adding intellisense comments, fixing the testing framework, possibly introducing a umd wrapper and making the library nodejs compatible. We had agreement - about 3 weeks ago in conference call - to proceed on the condition that I preserve the IE8/IE9 behaviour in a legacy branch. Are you saying that this is now not possible?

[vbhayden]

It's possible and it's been discussed more than once internally, but it's still undecided what the best approach would be for that.

There are benefits and an elegance to shimming this browser wrapper to achieve it, but it also introduces a fair bit of risk and goes beyond this project's original scope of providing a simple interface with an LRS from a browser.

Whether it's handled that way, through a new project submoduling the browser version, or an entirely node-native npm module sitting on axios is up for discussion. It's something people seem to want, so we're all ears for suggestions.

[oliverfoster]

I have a road map to achieve those outcomes with this library. I have resourced the time to complete it and I was operating under the assumption that I had preliminary agreement from your side.

Having already reworked this library a few times to achieve the above goals, I cannot see that there would be any adverse risks to the code base or its performance from making these changes. It all looks relatively straight forward once IE10 is the lowest browser.

As ADL has a reputation for being the central place for all things xAPI, it makes a lot of sense to broaden and sharpen the scope of this library somewhat (remove the individual file use in favour of a single compiled library), by having one ES wrapper, hosted in the ADL repo, which is cross environment (requirejs/commonjs/typescript defined/nodejs/browser), well documented, well tested and adheres to good coding norms.

I have already been in-touch with the owner of the doxstrap library to fix the library such that it now works for this project. I have explored and documented parts of the code base whilst formulating and building out the tests.

I have a path forward which I had explained in some detail on the conference call and which I am happy to explain again. I have tried to attend your weekly group meeting for the last 2 weeks to seek further clarification and guidance but the meeting has not commenced on either week.

I will finish this pr and ensure its safe passage into the code base but will cease any further works until agreement can be reached.

[oliverfoster]

Did you manage to review the last commits on this pr? @vbhayden

[vbhayden]

Merged per the conversation yesterday.

[oliverfoster]

Thanks Trey.