Skip to content

Verify event is not null on touch.js#13

Open
as0ler wants to merge 2 commits intonowsecure:mainfrom
as0ler:fix/verify_event_not_null
Open

Verify event is not null on touch.js#13
as0ler wants to merge 2 commits intonowsecure:mainfrom
as0ler:fix/verify_event_not_null

Conversation

@as0ler
Copy link

@as0ler as0ler commented Mar 3, 2023

No description provided.

Copy link
Contributor

@mrmacete mrmacete left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@oleavr oleavr left a comment

Choose a reason for hiding this comment

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

Yay, great catch!

app.sendEvent_(event);
} finally {
pool.release();
if (event !== null) {
Copy link
Member

@oleavr oleavr Mar 3, 2023

Choose a reason for hiding this comment

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

Oh wait, sorry, I'm not sure I understand what happens here. Does -[UIApplication _touchesEvent] keep returning nil or is it a random glitch? If it's the latter I'm wondering if we should make the _touchesEvent() call earlier, before we do pending.shift(), and simply return there -- that way dispatchTouch: gets called again on the next frame, and might succeed that time and keep going. (But if this is a permanent failure, I suppose we should call onComplete() with an Error so it can reject the Promise?)

Copy link
Author

Choose a reason for hiding this comment

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

It occurs when the application goes to the background for any reason and comes back. So I would say it's happening on a non-predictable manner.

Copy link
Member

@oleavr oleavr Mar 3, 2023

Choose a reason for hiding this comment

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

Interesting. I guess the tap down event instantly triggers backgrounding, and in this failure case we get the next dispatchTouch: just after the app has gone off screen -- whereas in the happy case we get it after it is back on the screen again.

If that is correct, I think we have two options:

  1. Keep trying, by returning early, before we do pending.shift(). In this way we will eventually move it through the different phases.
  2. Complete the touch right away, as if pending.length === 0. In that case we will have to release priv.touch if we already created it.

It seems to me that 1) is the right solution, since we will keep going once the app is back on the screen. This is also what happens when we are luckier -- the app goes to the background and dispatchTouch: does not get called until it's back on the screen.

What do you think, @mrmacete?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants