Skip to content
This repository has been archived by the owner on Dec 14, 2021. It is now read-only.

Fix CustomEvent Implementation #152

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

WebReflection
Copy link

@WebReflection WebReflection commented Dec 4, 2018

Currently new CustomEvent('!').constructor === CustomEvent returns false in IE11, which is a hell of a bummer for anything else that is either polyfilling or feature detecting CustomEvent after this file is on the page.

Bonus: I have removed the redundant in check when typeof is all it's needed.

Please consider publishing this fix ASAP since it's conflicting with other polyfills.

Thank you.

dom.js Outdated
return;
function CustomEvent ( event, params ) {
params = params || { bubbles: false, cancelable: false, detail: undefined };
var evt = document.createEvent( 'CustomEvent' );
var evt = creteCustomEvent();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the typo on purpose?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gosh no

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as in ... squashed now

Currently `new CustomEvent('!').constructor === CustomEvent` returns false, which is a hell of a bummer for anything else that is either polyfilling or feature detecting CustomEvent after this file is on the page.

Bonus: I have removed the redundant `in` check when `typeof` is all it's needed.

Please consider publishing this fix ASAP since it's conflicting with other polyfills.

Thank you.
@Mouvedia
Copy link

Mouvedia commented Dec 4, 2018

Please consider publishing this fix ASAP since it's conflicting with other polyfills.

You will have to wait for @inexorabletash's review.

@WebReflection
Copy link
Author

will that ever happen?

@inexorabletash
Copy link
Owner

Please add a test case, then I can merge.

@WebReflection
Copy link
Author

The test case is in the ticket:

Currently new CustomEvent('!').constructor === CustomEvent returns false in IE11

I'm on vacation 'till 26th so I won't change this PR before that day.

@Mouvedia
Copy link

Mouvedia commented Dec 11, 2018

Common @WebReflection just add one line there if you don't want to create another test case.

@WebReflection
Copy link
Author

I'm not lazy, I'm on vacation without internet and my laptop not always with me.

@inexorabletash
Copy link
Owner

With the patch applied, this fails in IE11:

assert.ok(new CustomEvent('!').constructor === CustomEvent);

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.

None yet

3 participants