1930374 - There is no response when clicking the close button for the first time in touch mode

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect, P3)

Description

[ahaibgp545]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:134.0) Gecko/20100101 Firefox/134.0

Steps to reproduce:

Firefox has no response when clicking on the X number in the upper right corner with your finger. You need to click again to close the browser. Mouse function is normal
Firefox Nightly 134-01a 2024-11-9

Actual results:

Firefox has no response when clicking on the X number in the upper right corner with your finger. You need to click again to close the browser. Mouse function is normal

Expected results:

Close the browser at first time when i touch by finger

Comment 1

[ahaibgp545]

Touch mode didnt well

Comment 2

[ahaibgp545]

Attachment: EasiRecorder_2024-11-10_21-31-04.mp4

cant move window by finger too. click the textinput, the virtual key board didnt appear.

Comment 3

[ahaibgp545]

In the title bar

Comment 4

[Francesco Lodolo [:flod]]

Not sure if there's a specific component for touch UI, in case feel free to move.

Comment 5

[ahaibgp545]

Will it support Touch ui in the future?

Comment 6

[ahaibgp545]

I hope that Firefox will adapt the touch UI. Its really important for some users.Edge and Chrome has adapted the TouchUI.

Comment 7

[Ray Kraesig [:rkraesig]]

I can repro on Win10 using the current Release version.

Comment 8

[Ray Kraesig [:rkraesig]]

Okay, I think I know at least part of what's happening, although not why.

When the first tap occurs, it's hit-detected as an HTCLOSE; Windows accordingly generates a WM_NCLBUTTONDOWN (rather than a WM_LBUTTONDOWN). This gets dispatched as a DOM event, bypassing APZ, although it doesn't have any visible effect (not even a highlight). It's not surprising that the window doesn't close yet -- that should happen in a moment, when the corresponding WM_LBUTTONUP comes through...

... except that the WM_LBUTTONUP is intercepted by the APZ code and promptly discarded. This actually leaves Firefox in a (temporary) bad state: trying to drag it around via touch on the tab-bar does nothing!

The easy fix — which almost certainly breaks something else — is to add ..., true) to the DispatchMouseEvent call for WM_LBUTTONUP. This doesn't immediately break pinch-to-zoom, but that's the only test I've got in my pocket.

Forwarding to Core :: Panning and Zooming for further consideration: what should be happening here, from APZ's perspective?

Comment 9

[Botond Ballo [:botond]]

(In reply to Ray Kraesig [:rkraesig] from comment #8)

what should be happening here, from APZ's perspective?

I haven't come across this aIgnoreAPZ check before; it looks like it was added by :handyman in bug 1736128, he might have the most context on this question. If not, please feel free to bounce this back to me and I'll do some code reading to brush up on my understanding of these Windows widget functions.

Comment 10

[Ray Kraesig [:rkraesig]]

(In reply to Botond Ballo [:botond] from comment #9)

(In reply to Ray Kraesig [:rkraesig] from comment #8)

what should be happening here, from APZ's perspective?

I haven't come across this aIgnoreAPZ check before; it looks like it was added by :handyman in bug 1736128, he might have the most context on this question. If not, please feel free to bounce this back to me and I'll do some code reading to brush up on my understanding of these Windows widget functions.

:handyman's out on PTO at the moment, but it's clear enough from bug 1736128 that aIgnoreAPZ exists to keep APZ from eating the WM_NCLBUTTONDOWN event. It is not set for the WM_LBUTTONUP event.

I'm less interested in that than in the return result, which has been in there since bug 1144650 — which is described in the git summary as "Don't dispatch touch-based mouse events when APZ is handling touch." Unfortunately, APZ appears not to actually be handling this touch event.

Comment 12

[Botond Ballo [:botond]]

We discussed this over Matrix, but to summarize:

• The codepath where we "eat" a mouse event (do not create a WidgetMouseEvent for it and dispatch it to Gecko) is intended for cases where we previously received a touch event for the same user interaction, and dispatched that to Gecko (as a MultiTouchInput, around here). The handling of the touch events ensures that e.g. a click (tap) is correctly processed.
• For touch interactions over the non-client area, the native touch events are of a different type (NCPOINTER rather than POINTER) which we don't handle. To ensure that we react to these interactions, we need to either:
  • Ensure we do handle the corresponding mouse events. This is what bug 1736128 did for the NCLBUTTONDOWN, but for the scenario in this bug we'd need to extend that handling to the corresponding button-up (which for some reason is an LBUTTONUP rather than NCLBUTTONUP).
  • Or alternatively, start handling the native touch events (e.g. NCPOINTER) themselves (in which case the workaround added in bug 1736128 can likely be removed).

Comment 13

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1930374 - [1/4] Drive-by: silence ResolveJunctionPointsAndSymLinks messages r?#win-reviewers

These messages aren't really Widget-relevant, even if they are
Windows-OS-relevant. Push them off into their own logging channel.

Comment 14

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1930374 - [2/4] Refactor ClientMarginHitTestPoint r?#win-reviewers

Make the early exit actually look like an early exit.

No functional changes.

Comment 15

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1930374 - [3/4] Refactor WM_NCLBUTTONDOWN handler r?#win-reviewers

Minor cleanups to the WM_NCLBUTTONDOWN handler:

• Use the hit-test value passed in as wParam instead of calling
  ClientMarginHitTestPoint again.
• Change the final boolean parameter on DispatchMouseEvent (only used
  here, so far) to a named-boolean, to disambiguate at the call-site.

No functional changes.

Comment 16

[Ray Kraesig [:rkraesig]]

Attachment: Bug 1930374 - [4/4] Avoid capturing mouse on nonclient mousedowns r?#win-reviewers!

If we capture the mouse on receipt of a WM_NCLBUTTONDOWN message, the
subsequent mouse-up message will actually be a WM_LBUTTONUP rather than
a WM_NCLBUTTONUP. (This behavior is reported to go back to at least
Win95 and NT 3.51.)

We could handle that... but it's easier not to have to; and we don't
actually need to capture the mouse when the user clicks on one of the
window-control buttons.

(We do, however, need to handle the WM_NCLBUTTONUP messages that this
causes us to start receiving.)

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2f9bbffec31a
[1/4] Drive-by: silence ResolveJunctionPointsAndSymLinks messages  r=win-reviewers,handyman
https://hg.mozilla.org/integration/autoland/rev/ef82e62ee6a1
[2/4] Refactor ClientMarginHitTestPoint  r=win-reviewers,handyman
https://hg.mozilla.org/integration/autoland/rev/2cf5ffcc03b9
[3/4] Refactor WM_NCLBUTTONDOWN handler  r=win-reviewers,handyman
https://hg.mozilla.org/integration/autoland/rev/5b24933e8b3d
[4/4] Avoid capturing mouse on nonclient mousedowns  r=win-reviewers,handyman

Comment 18

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/2f9bbffec31a
https://hg.mozilla.org/mozilla-central/rev/ef82e62ee6a1
https://hg.mozilla.org/mozilla-central/rev/2cf5ffcc03b9
https://hg.mozilla.org/mozilla-central/rev/5b24933e8b3d