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

addEventListener polyfill is firing the actual event on the actual element the first time that event is listened to #18

Closed
jpray opened this issue Aug 17, 2015 · 8 comments

Comments

@jpray
Copy link

jpray commented Aug 17, 2015

Line 353 is
self.fireEvent(ontype, e);

In my case, this is submitting a form as soon as the listener gets added. I see in the comments that you want to perform the test on the actual element, but maybe there's a way to silence the event so it doesn't propagate or trigger the default behavior?

@jpray
Copy link
Author

jpray commented Aug 17, 2015

Just a bit more information for anyone that's interested. I'm using (having to use) jQuery, and my current workaround is to include the following code block directly after including ie8.js.

$('<form></form>').submit(function(){});

@WebReflection
Copy link
Owner

as you can read here the whole point is indeed to silent the test, you have conflicts brought it by jQuery.

I don't know why you would use this polyfill if you use jQuery, but I am not actually planning to fix all problems that third parts libraries might cause out there (as matter of time, mostly)

This polyfill is about DOM standards, are you sure you just don't want to use jQuery 1.X instead?

If you have to support IE8 and you want to use jQuery it seems like a better answer.

[edit] trying to see if I can at least cancelBubble if there is a SECRET

@WebReflection
Copy link
Owner

Can you please try this version and tell me if with that you still need the jQuery hack?

Maybe it was simpler than expected, Thanks

@jpray
Copy link
Author

jpray commented Aug 17, 2015

Firstly, thank you for this lib, dom4.js, and document-register-element.js. None of us dev's choose to support IE8 for so long, but such is life, and your libs make it a lot more bareable...

I was able to reproduce the issue without using jQuery using the following page

<!DOCTYPE html>
<html lang="en-us">
<head>
    <!--[if lt IE 9]><script src="ie8.max.js"></script><![endif]-->  
</head>

<body>
    <form id="testForm" onsubmit="alert('submitted')" action="post.php">
        <input type="text">
        <button type="submit" value="submit">Submit</button>
    </form>
    <script type="text/javascript">
        document.getElementById('testForm').addEventListener('submit',function(){});
    </script>

</body>
</html>

In this test page, the onsubmit method is still being invoked with the updated build. If it is possible to fix this, the use case that it benefits is when someone is working with a legacy codebase but trying to add all new features in a more standards-based way. In that case there might be inline javascript and/or jQuery hanging around until those parts get refactored. If a fix is possible, AWESOME, it means I can drop this lib into a legacy codebase and start using standards. If a fix isn't possible, then it would probably at least be good for people to know what to look out for when adding this lib to a legacy codebase. Maybe it just warrants a mention in the known gotchas section of the readme...

Thanks for your time and everything.

@WebReflection
Copy link
Owner

uhm ... <form id="testForm" onsubmit="alert('submitted')" action="post.php"> is bad technique but I understand the legacy concern.

Yeah, if you don't use standards to add events, ie8 and dom4 might fail in many ways. These libraries have been created to actually let people write standards, not as migration to standard patterns, specially because ie8 + attachEvent on top of legacy might do more damage than solving.

The migration should be a proper refactoring, but I understand this might be not reasonable, as task to waste time, in some bloated, badly architected, legacy code.

I'll try to think about some way to avoid the inline / onpage event triggering, but I will need you to test again your legacy.

@WebReflection
Copy link
Owner

the form page has been tested against IE8 ( through Windows XP VM ) and it seems to work as expected: no regressions, and no alert shown.

Can you please confirm the latest build works?

@WebReflection
Copy link
Owner

OK, I have done other enhancement. The current version 0.2.8 should avoid the mentioned problem.

I will close this, but feel free to ask me more, if needed.

Cheers

@jpray
Copy link
Author

jpray commented Aug 18, 2015

Thank you very much! Cheers...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants