1557710 - It is very difficult to resize by dragging top border of browser if titlebar is disabled

Status: VERIFIED FIXED

(Core :: Widget: Win32, defect, P2)

Description

[Alice0775 White]

+++ This bug was initially created as a clone of Bug #1549972 +++

Sometimes, the mouse pointer is changed to resizer cursor. But the pointer will be immediately changed to normal cursor.

See screencast: attachment 9070569 [details] bad(Nightly69.0a1) vs good().mp4
00:00 - 00:14 Nghtly69.0a1 --- Bad
00:15 - 00:28 Firefox67.0 --- Good

If titlebar is enabled(customize and checked TitleBar), the problem does not happen.

Steps To Reproduce:

1. Disable titlebar (this is default)
2. Try to resize the window by dragging the top border of browser

Actual Results:
Sometimes, the mouse pointer is changed to resizer cursor. But the pointer will be immediately changed to normal cursor.

Expected Results:
The mouse pointer is changed to resizer cursor at the top edge. And the cursor shape persists as long as you don't move the mouse pointer.

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=32552726554321d35a8c18224343fe73ece324c2&tochange=0930049c0ca984c909d11ba2853ef6d96e43e242

Regressed by: 0930049c0ca984c909d11ba2853ef6d96e43e242 Mike Conley — Bug 1534389 - Send normal mouse events when cursor is over a draggable region on Windows. r=jmathies

And unfortunately Bug 1549972 did not fix this issue.

Comment 1

[Alice0775 White]

The offending patch was backed out from 68beta.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Hi Alice,

Can you confirm that the latest beta no longer has the regression?

Comment 3

[Alice0775 White]

(In reply to Mike Conley (:mconley) (:⚙️) from comment #2)

Hi Alice,

Can you confirm that the latest beta no longer has the regression?

Yes, I can confirm that 68.0b13 fixes the regression.

Version 68.0b13
Build ID 20190624133534
User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Firefox/68.0

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Hi Alice0775,

Thanks for confirming. Can you try testing something else for me? Does this build re-introduce the problem for you?:

32-bit: https://queue.taskcluster.net/v1/task/S3udqAWIT-aH1T9XxUqlyw/runs/0/artifacts/public/build/target.zip
64-bit: https://queue.taskcluster.net/v1/task/M9WdrCiyQeaZTjngnQ5SSg/runs/0/artifacts/public/build/target.zip

Comment 5

[Alice0775 White]

Attachment: screen cast

(In reply to Mike Conley (:mconley) (:⚙️) from comment #4)

Hi Alice0775,

Thanks for confirming. Can you try testing something else for me? Does this
build re-introduce the problem for you?:

32-bit:
https://queue.taskcluster.net/v1/task/S3udqAWIT-aH1T9XxUqlyw/runs/0/
artifacts/public/build/target.zip
64-bit:
https://queue.taskcluster.net/v1/task/M9WdrCiyQeaZTjngnQ5SSg/runs/0/
artifacts/public/build/target.zip

Seems nothing is changed.
The problem is still reproduced.
The mouse pointer is changed to resizer cursor. But the pointer will be immediately changed to normal cursor.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Hey Cristian, are you able to reproduce Alice0775's issue at all? I'm having a heck of a time doing it, and I'm wondering what I'm doing wrong - both Firefox Release and Firefox Nightly behave the same way for resizing at the top of the window for me.

Comment 7

[Cristian Craciun]

Build ID 20190627093636
User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:69.0) Gecko/20100101 Firefox/69.0

Hey Mike, I've tested this issue but didn't managed to reproduce it. I've tested on the latest Nightly build on Windows 10 both 32-bit and 64-bit versions of the browser. Should I try to reproduce it on another version of the browser?

Comment 8

[Mike Conley (:mconley) (:⚙️)]

I'm not sure - your experience matches mine, but check out Alice0775's videos - something strange is going on. I think we have to figure out why Alice0775's experience is different.

Alice0775, can we assume that you're still able to reproduce this on Nightly? Also, are you doing these tests on new profiles? Finally, which version of Windows 10 are you running? Do you have any customizations to your mouse cursors, or perhaps a non-default desktop scaling?

Comment 9

[Alice0775 White]

(In reply to Mike Conley (:mconley) (:⚙️) from comment #8)

I'm not sure - your experience matches mine, but check out Alice0775's
videos - something strange is going on. I think we have to figure out why
Alice0775's experience is different.

Alice0775, can we assume that you're still able to reproduce this on
Nightly?

Yes, still reproduce on latest Nightly69.0a1 Build ID 20190627093636.

Also, are you doing these tests on new profiles?

Yes, Of course, I tested clean new profile.

Finally, which version of Windows 10 are you running?

Windows10 Home, Version 1903 build 18362.175.
Windows10 Pro Workstation, Version 1809 build 17763.437.

Do you have any customizations to your mouse cursors, or perhaps a non-default desktop scaling?

No, I re-tested windows10 default theme and default Mouse pointer. Same thing happened.
And also I installed new Windows10 pro on VMWare guest. Same thing happened.

Comment 10

[Alice0775 White]

Attachment: the area of problem

When I move mouse pointer upward from tabstrip to outside of browser.
The mouse cursor changed the following sequence.

on tabstrip -> just below border -> on border(+a pixel outside)
Normal arrow -> Resizer, immediately after Normal arrow -> Resizer

Comment 11

[Itiel]

I can reproduce what Alice is seeing, with the help of the screenshot above.
Though I can still resize the window afterwards (after the cursor changes to the normal one, clicking and dragging the border will resize the window).

Comment 12

[Mike Conley (:mconley) (:⚙️)]

So I think I've more or less figured out what's going on here.

In bug 1534389, I made it so that we fire dispatch mouse move events down into Gecko if we detect mouse moves within a draggable region - this was so that we could get the Picture-in-Picture controls to display on hover when mousing over it (since the whole thing is more or less a draggable region).

I think we're seeing a tug-of-war over who can set the mouse cursor - the tug-of-war is between EventStateManager / PresShell and widget/windows/nsWindow.cpp.

Let's suppose that the mouse is at the top of the window, over a draggable region, and just about to enter the point where we expect the resize cursor to appear. Let's say we're 1 pixel away, so if we move the mouse up 2 pixels, we expect to transition from the default cursor to the resize cursor.

First pixel

The widget layer first detects the mouse movement, and does a hittest via the WM_NCHITTEST message. The hittest resolves to HTCAPTION, so Windows interprets the result of the WM_NCHITTEST message as, "you're in a draggable region". So at this point, we still show the default pointer cursor.

Next, the WM_NCMOUSEMOVE event comes in. Thanks to my code in bug 1534389, since we determine that we're within a draggable region, we re-dispatch that as a normal WM_MOUSEMOVE message. This causes the mouse movement to be turned into a Gecko event, and sent down to the EventStateManager.

This next part is kinda timing dependent, and also where my understanding gets a bit fuzzy...

PresShell seems to have this notion of a synthesized mousemove event... I believe this is in the event that the underlying structure of the page changes without the mouse actually moving. Anyhow, one of those synthetic mouse move events seems to get queued up via PresShell::DidDoReflow.

So at this point, we have our synthetic mousemove all ready to go, waiting for FlushType::Display to fire.

Second pixel

The mouse moves again. The widget layer first detects the mouse movement, and does a hittest via the WM_NCHITTEST message. The hittest resolves to HTTOP, so Windows interprets the result of the WM_NCHITTEST message as, "you're at the top of a resizable window". So at this point, we show the resize cursor. Hooray!

And then our synthetic mousemove event is handled. The EventStateManager handles that synthetic event, and tells the widget layer to update the cursor again - this time, to the default cursor (the pointer), because the frame that the mouse is over (presumably) doesn't have a cursor CSS rule set.

So then the cursor is set back to the pointer.

Thanks to the hittest, clicking still causes the resize action to work, but the mouse cursor doesn't really communicate this.

I think this is what's happening, and I think the reason that this is a problem now is that synthetic mouse events weren't ever in conflict like this with the widget hittest stuff. However, now that we're sending down WM_MOUSEMOVE events to Gecko when mousing over draggable regions with WM_NCMOUSEMOVE... the bug has kinda been revealed.

Comment 13

[Mike Conley (:mconley) (:⚙️)]

Hi Alice0775, can you help me determine if what I've found here is related to what you're still seeing?

Can you go to about:config, and set layout.reflow.synthMouseMove to false, and restart the browser, and see if you can reproduce the issue? If not, then I think we've more or less narrowed down to our culprit here. (Don't forget to reset that pref after testing)

Comment 14

[Mike Conley (:mconley) (:⚙️)]

Presuming I've centered in on the problem here, I'm not 100% sure what the right solution here is. Two things come to mind:

1. Don't dispatch the synth mousemove event if we know the mouse is in the special window edge region as defined by this constant. I guess that would mean implementing something on nsIWidget, and calling into it from PresShell to check whether or not we need to dispatch the event inside of ProcessSynthMouseMoveEvent.

2. Don't update the cursor if we know the mouse is inside of the special window edge region as defined by this constant. This is similar to (1), but allows the synth mousemove event to be dispatched - we just skip this update right here.

3. Expose a way of revoking the synthesized mouse move event publicly, so that if we hittest on any of our special window edge regions, we can revoke it.

But I'm open to suggestions on other alternatives.

Hey tnikkel, I see your name on some of this synth mousemove stuff... are you the right person to ask which way I should probably proceed on this?

Comment 15

[Timothy Nikkel (:tnikkel)]

(In reply to Mike Conley (:mconley) (:⚙️) from comment #14)

Presuming I've centered in on the problem here, I'm not 100% sure what the
right solution here is. Two things come to mind:

They all sound okay to me actually.

Comment 16

[Alice0775 White]

(In reply to Mike Conley (:mconley) (:⚙️) from comment #13)

Hi Alice0775, can you help me determine if what I've found here is related
to what you're still seeing?

Can you go to about:config, and set layout.reflow.synthMouseMove to
false, and restart the browser, and see if you can reproduce the issue? If
not, then I think we've more or less narrowed down to our culprit here.
(Don't forget to reset that pref after testing)

After create the pref and set it to false and restart browser, the problem is not reproduced.

Comment 17

[Mike Conley (:mconley) (:⚙️)]

Thanks, Alice0775 White and tnikkel.

It turns out we already have a built-in mechanism for ignoring these synthetic mouse movements! It looks like if I send in a "mouse exited the widget" message (which is what we used to fire when entering the non-client area), we'll run this code which sets the PresShell's notion of the mouse to (NS_UNCONSTRAINEDSIZE, NS_UNCONSTRIANEDSIZE). This causes us to ignore any queued up synthetic mouse moves here.

So I think we just need to make sure we fire the "mouse exited from widget" message when we notice the mouse has moved out from the draggable area to any other non-draggable non-client region. Patch coming up.

Comment 18

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1557710 - Fire eMouseExitFromWidget when transitioning from a draggable to a non-draggable non-client region on Windows. r?jmathies

We need to do this in order to not override the mouse cursor that we set in the
widget layer when hit-testing in the non-client region.

We were accidentally overriding before because the PresShell normally queues up a
synthetic mousemove event when the mouse is moving within the client region. That
mousemove cause the EventStateManager to update the cursor to match Gecko's
reckoning of the cursor CSS style of the underlying frame (which overrides the
cursor we may have set in the Windows non-client region hittest - for example,
one of the window resize cursors).

By clearing the mMouseInDraggableArea boolean when transitioning from a draggable
to non-draggable region in the non-client region, we make sure that we process the
WM_MOUSELEAVE message, which sends the eMouseExitFromWidget event into Gecko, which
effectively cancels handling of the synthetic mousemove.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/db37a1fb11a1
Fire eMouseExitFromWidget when transitioning from a draggable to a non-draggable non-client region on Windows. r=jmathies

Comment 20

[Arthur Iakab [arthur_iakab]]

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

Comment 21

[Alice0775 White]

I verified that the issue was fixed on latest Nightly70.0a1 Windows10.
Build ID 20190710215049
User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:70.0) Gecko/20100101 Firefox/70.0

Comment 22

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 9076811 [details]
Bug 1557710 - Fire eMouseExitFromWidget when transitioning from a draggable to a non-draggable non-client region on Windows. r?jmathies

Beta/Release Uplift Approval Request

• User impact if declined: Users may find it more difficult than usual to resize Firefox windows on Windows via the top part of the window.
• 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): The change I introduced causes us to much more closely match what we've done and shipped for years.
• String changes made/needed: None.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9076811 [details]
Bug 1557710 - Fire eMouseExitFromWidget when transitioning from a draggable to a non-draggable non-client region on Windows. r?jmathies

Fixes a regression in 69 making it harder for Windows users to resize windows in some cases. Approved for 69.0b5.

Comment 24

[Razvan Maries]

https://hg.mozilla.org/releases/mozilla-beta/rev/3c3a566d8335