fix: Check HTMLDocument constructor exists first for IE10

[junghwan-park]

AS-IS

• IE10 is supported browser. But HTMLDocument constructor is not exists in IE10.
• fabric.js bundle file using HTMLDocument constructor for check document is instance of it.

TO-BE

• Use Document constructor instead of HTMLDocument constructor if HTMLDocument is not exists in browser global variable.

Resolution

• Check HTMLDocument exists first.
  • If not, Use Document for check document is instance of it.

ref: #5677

[asturur]

@ajvincent can you verify this change does not kill XML document support please? i do not have time for it.

[asturur]

@junghwan-park thanks ot be proactive with the fix

[ajvincent]

Apologies for not responding yet: I have been rather busy myself. I have this ticket pinned in my browser to remind me to test it, but on the surface it looks fine.

[ghost]

@asturur I have build @junghwan-park branch (https://github.com/junghwan-park/fabric.js/tree/fix-ie10-support) and it looks fine, so I will wait until this is merged.

[ajvincent]

The proposed change does pass the editor.zip test I posted in #5530, but on second look at the patch included, I have a minor concern.

Specifically, the code adds a top-level var DocumentConstructor. Think about namespace pollution, or just being nice to neighboring code in the same scope.

If this block were enclosed in a self-executing function which then defined the necessary properties, I'd be fine with this. That said, be careful how much you enclose in such a function: DOMParser = require('xmldom').DOMParser; is also in the HEADER.js file in the node.js path, and turns up much later.

Consider this a peanut-gallery code review: if @asturur wants to accept the patch as-is, or require a change similar to what I suggest, that is his decision.

[asturur]

Your review is what i was looking for actually. I did not notice/remember the header wasn't wrapped in an anti-leak function wrapping, so that VAR cannot be there at all.
Thanks for spotting this and thanks for verifying that editor.zip works. I really had no time.

[asturur]

@junghwan-park sorry for the delay, can you look at the comment/discussion?

thanks for opening this!

[junghwan-park]

@asturur I've changed var definition into ternary operation.

[asturur]

Guys thanks for collaboration

[junghwan-park]

@asturur @ajvincent Thanks.
I'll looking forward to this feature releasing. :)

[asturur]

i ll merge another pr and release 3.0.1