1736128 - Cannot use touchscreen input to close, maximize, or minimize a Firefox window

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Dennis Schubert [:denschub]]

Attachment: A screen recording showing the unresponsive window controls

I have recorded a screencast of the issue showing the touch inputs, see the attachment.

This is on Windows 11, on a Surface Pro 7+. I unfortunately don't know if this only affects Windows 11, or if Windows 10 is also affected, since I only have one device to test with. Pen inputs and attached keypads work fine.

mozregression told me that this is a regression of Bug 1730309 - which unfortunately also got uplifted into Beta.

[Tracking Requested - why for this release]:

This breaks the window management for Touchscreen users on Windows, see the attached screencast.

Comment 1

[Dennis Schubert [:denschub]]

Also ni? :cmartin because David Parks is away, apparently.

Comment 2

[David Parks [:handyman]]

(I have been away for a few days without a bugzilla name change since I've been working with intern candidates while I'm out and I didn't want to confuse them. Apologies for confusing you instead. I'm back on Monday anyway.)

I'm able to reproduce this with the Oct 15 Nightly in a Windows 11 VM and in Windows 10 (not a VM) on a Surface Pro. Bug 1730309 was actually extending the maximize button behavior added in bug 1718629 to the minimize and close buttons. Mozregression confirms that bug 1718629 introduces this issue for the maximize button.

My first guess is that this is swallowing the touch event instead of giving it back to Windows, where it would be re-passed as a WM_NCHITTEST followed by a WM_NCLBUTTONDOWN (whose handler would also return the event to Windows, causing Windows to handle it itself). Before bug 1718629 / bug 1730309, the window buttons were handled by Firefox instead of by Windows, so we didn't want to return the event. IOW, I'm guessing nsWindow::OnPointerEvents should be returning false when IsWindowButton(ClientMarginHitTestPoint(GET_X_LPARAM(lParam), GET_Y_LPARAM(lParam))) returns true. It won't try to do the hit test multiple times (under normal behavior) because ClientMarginHitTestPoint caches its result.

Comment 3

[David Parks [:handyman]]

It turns out the situation is caused by thread state that Windows maintains (GetMessageExtraInfo). It is tested by WinUtils functions that are used in the DispatchMouseEvent call that was added to the WM_NCLBUTTONDOWN handler (and some other places). The global state tells things that the "synthesized" mouse event came from a pointer. The event isn't really "synthesized" -- it was just from a WM_NCLBUTTONDOWN (not a WM_POINTERDOWN) instead of a WM_LBUTTONDOWN -- but the thread state screws that up.

We can circumvent the stuff that uses the invalid state pretty easily but we should try to find why it is making a difference to see if there is a more direct fix first.

FYI, the WM_POINTER* messages were not relevant because there are WM_NCPOINTER* messages for the non-client area.

Comment 4

[David Parks [:handyman]]

Attachment: Bug 1736128: Ignore APZ detection in Windows touch event sent for window buttons r=emk!

Since we don't handle WM_NCPOINTER* touch events, Windows resends them as WM_NCLBUTTON* events but leaves the special touch-indicators set in thread extra data (::GetMessageExtraInfo). This extra data causes our event to be ignored as a "touch-generated mouse event" that isn't part of an APZ drag. This patch skips that check for these specific window-button mouse events.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d4833204f61e
Ignore APZ detection in Windows touch event sent for window buttons r=emk

Comment 6

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/d4833204f61e

Comment 7

[David Parks [:handyman]]

Comment on attachment 9246480 [details]
Bug 1736128: Ignore APZ detection in Windows touch event sent for window buttons r=emk!

Beta/Release Uplift Approval Request

• User impact if declined: The regressing bug(s) have the effect of blocking touch screen users from being able to use the window management buttons (minimize, maximize and close). This patch restores the functionality.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This patch exclusively applies to a synthesized mouse event generated for the case of a window button press on a touch screen. This mouse event was previously ignored but is not after this patch. The behavior now mirrors use that is already live in the case of a mouse click.
• String changes made/needed: N/A

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9246480 [details]
Bug 1736128: Ignore APZ detection in Windows touch event sent for window buttons r=emk!

Approved for 94.0b8.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/9c810a23a325

Comment 10

[Ciprian Georgiu, Desktop QA]

Reproduced this issue on an affected Nightly build, using a Dell XPS with touchscreen.

The issue is verified as fixed on the latest builds: Nightly 95.0a1 and Beta 94.0b8.