Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug in modal.js: defaultPrevented called on jQuery.Event instead of isDefaultPrevented() #31615

Closed
joakimriedel opened this issue Sep 9, 2020 · 6 comments · Fixed by #31696
Closed
Labels

Comments

@joakimriedel
Copy link

Tried to prevent the hidePrevented.bs.modal event using e.preventDefault(), but it won't work since the code checks for defaultPrevented instead of isDefaultPrevented().

The bug on this line: https://github.com/twbs/bootstrap/blob/v4.5.2/js/src/modal.js#L237

All the other events are properly using isDefaultPrevented() see for example this line: https://github.com/twbs/bootstrap/blob/v4.5.2/js/src/modal.js#L117

Here's a simple fix: https://github.com/twbs/bootstrap/compare/v4.5.2...joakimriedel:joakimriedel/defaultPrevented?expand=1

Patch for 4.5.2 ?

@joakimriedel
Copy link
Author

As a hacky workaround, I now do Object.assign(e, { defaultPrevented: true }) together with e.preventDefault() to prevent the animation.

@XhmikosR XhmikosR added the js label Sep 18, 2020
@XhmikosR
Copy link
Member

@Johann-S can you have a quick look please? Also please add the v4/v5 labels if this applies to both versions.

@Johann-S
Copy link
Member

it's just in v4 here, in v5 we use defaultPrevented because we use Vanilla JS

@Johann-S Johann-S added the v4 label Sep 18, 2020
@XhmikosR
Copy link
Member

@joakimriedel can you please make a new PR against the v4-dev branch along with a test?

@joakimriedel
Copy link
Author

@XhmikosR sure #31696

@XhmikosR
Copy link
Member

Thanks for the bug report and the PR. #31696 was merged.

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

Successfully merging a pull request may close this issue.

3 participants