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 Android double-tap not working. #59760

Closed
wants to merge 1 commit into from

Conversation

madmiraal
Copy link
Contributor

#37158 added the event's button state to Android's onDoubleTap gesture processing. However, the button state is always zero. This broke double tapping (#8151, #46100, #46101).

As per the documentation, the onDoubleTap notification is "Triggered on the down event of second tap." Furthermore, since double-clicks are only detected on the left mouse button, even if the button state was correct, we can safely assume that the event pressed parameter will be true, button_index will be MouseButton::LEFT and button_mask will be MASK_LEFT.

Therefore this PR reverts #37158's addition of the button state to the Android onDoubleTap gesture processing.

Fixes #8151.
Provides a partial fix for #46100 and #46101 in as much as the output on Android will be

Event doubleclick = False, pressed = True, button_index = 1
Event doubleclick = False, pressed = False, button_index = 1
Event doubleclick = True, pressed = True, button_index = 1
Event doubleclick = False, pressed = True, button_index = 1
Event doubleclick = False, pressed = False, button_index = 1

vs the desktop output of:

Event doubleclick = False, pressed = True, button_index = 1
Event doubleclick = False, pressed = False, button_index = 1
Event doubleclick = True, pressed = True, button_index = 1
Event doubleclick = False, pressed = False, button_index = 1

In other words there is still an extra event, but at least there is a correct doubleclick event that can be detected appropriately.

@madmiraal madmiraal requested a review from a team as a code owner March 31, 2022 18:33
@m4gr3d m4gr3d requested a review from thebestnom March 31, 2022 21:01
ev->set_button_index(_button_index_from_mask(event_button_mask));
ev->set_button_mask(event_button_mask);
ev->set_pressed(true);
ev->set_button_index(MouseButton::LEFT);
Copy link
Contributor

Choose a reason for hiding this comment

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

Right click can also generate double click (tested 2 years ago on non stock rom 😅) need to account for that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure supporting a right-button double-click that requires a "non stock rom" to even be triggered is needed. Looking at the comments from #37158 (e.g. #37158 (comment) and #37158 (comment)) it appears as if you agree. Is a right-button double click needed for anything?

That been said, to get this to work properly (and properly fix #46100 and #46101) would require separating out the mouse double-clicks from the double-taps. I'm working on a PR to achieve that. In the meantime this PR provides a fix for the regression introduced by #37158.

@Calinou
Copy link
Member

Calinou commented May 4, 2022

Does this PR supersede #54225?

@madmiraal
Copy link
Contributor Author

Does this PR supersede #54225?

Yes.

@akien-mga akien-mga requested a review from a team May 5, 2022 07:45
@m4gr3d
Copy link
Contributor

m4gr3d commented Jun 27, 2022

@madmiraal Looks good; just need to be rebased prior to merging.

Copy link
Contributor

@m4gr3d m4gr3d left a comment

Choose a reason for hiding this comment

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

@madmiraal Nevermind the previous review.

I've tested the current logic on a stock Android 12 on a Pixel 5, and despite what the documentation states, double clicks from a connected bluetooth mouse is also handled by the GestureHandler.

It looks like the proper solution would be #54225 instead.

@madmiraal
Copy link
Contributor Author

I've tested the current logic on a stock Android 12 on a Pixel 5, and despite what the documentation states, double clicks from a connected bluetooth mouse is also handled by the GestureHandler.

This PR doesn't prevent double-clicks from working. It just assumes all double-clicks are left mouse button clicks.

@m4gr3d
Copy link
Contributor

m4gr3d commented Sep 6, 2022

Superseded by #65434

@m4gr3d m4gr3d closed this Sep 6, 2022
@madmiraal
Copy link
Contributor Author

I think it's a bit presumptuous to close this issue before #65434 has been approved and merged.

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

Successfully merging this pull request may close these issues.

Directories cannot be entered by selecting items from a FileDialog on Android
5 participants