Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix: Added checks before removing event listeners #7705

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Sanketjadhav31
Copy link

Resolves #618

Changes:

  • Fixed potential runtime errors by ensuring removeEventListener is only called when the target element and handler exist.
  • Added checks to prevent calling removeEventListener on undefined values in the main.js file.

Screenshots of the change:

Not applicable


PR Checklist

Copy link

welcome bot commented Apr 5, 2025

🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already. And be sure to add yourself to the list of contributors on the readme page!

@Sanketjadhav31
Copy link
Author

Hi ,
This PR resolves issue #5404. I made changes in the main.js file to fix the placement issue of textLeading() in the reference sidebar. Now it correctly links to the right function. Let me know if any improvements are needed. Thank you!

@ksen0
Copy link
Member

ksen0 commented Apr 7, 2025

Hi @Sanketjadhav31 can you explain how this PR relates to the original issue? Also, the original issue looks closed?

Regarding this PR: in which situation are those properties missing, that requires this checks? Additionally, rather than silently suppressing the error, could you log a more descriptive message?

Thank you!

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

Successfully merging this pull request may close these issues.

2 participants