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

Fix rare crash in event handler #7525

Merged
merged 4 commits into from
Jan 10, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
- Fixed a bug where meshes (or chunks of them) were always colored white, if they were loaded while the corresponding segmentation layer was disabled. [#7507](https://github.com/scalableminds/webknossos/pull/7507)
- Fixed a race condition when opening a short link, that would sometimes lead to an error toast. [#7507](https://github.com/scalableminds/webknossos/pull/7507)
- Fixed that the Segment Statistics feature was not available in the context menu of segment groups and in the context menu of the data viewports. [#7510](https://github.com/scalableminds/webknossos/pull/7510)
- Fixed rare bug which produced a benign error toast on some mouse interactions. [#7525](https://github.com/scalableminds/webknossos/pull/7525)
- Fixed a bug where dataset managers were not allowed to assign teams to new datasets that they are only member of. This already worked while editing the dataset later, but not during upload. [#7518](https://github.com/scalableminds/webknossos/pull/7518)

### Removed
Expand Down
10 changes: 6 additions & 4 deletions frontend/javascripts/libs/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -765,10 +765,12 @@ export function addEventListenerWithDelegation(
handlerFunc: (...args: Array<any>) => any,
options: Record<string, any> = {},
) {
const wrapperFunc = function (event: Event) {
// @ts-ignore
for (let { target } = event; target && target !== this; target = target.parentNode) {
// @ts-ignore
const wrapperFunc = function (this: HTMLElement | Document, event: Event) {
for (
let { target } = event;
target && target !== this && target instanceof Element;
Copy link
Member

Choose a reason for hiding this comment

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

d.h. wenn target === document ist, dann wird der loop nicht ausgeführt / zum nächsten gesprungen?

Copy link
Member

Choose a reason for hiding this comment

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

In der PR Beschreibung geht es noch um window, dass wird hier nicht berücksichtigt, oder?

Copy link
Member Author

Choose a reason for hiding this comment

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

d.h. wenn target === document ist, dann wird der loop nicht ausgeführt / zum nächsten gesprungen?

wenn target === document und this === document ist, wird die loop abgebrochen. in dem fall wurde dann kein passendes element gefunden, auf dem der event listener triggern sollte. das verhalten sollte also in ordnung sein.

In der PR Beschreibung geht es noch um window, dass wird hier nicht berücksichtigt, oder?

sowohl window als auch document sind nicht instance of Element. diese beiden fälle werden durch den instanceof check abgedeckt.

target = target.parentNode
) {
if (target.matches(delegateSelector)) {
handlerFunc.call(target, event);
break;
Expand Down