Closed Bug 632953 Opened 9 years ago Closed 9 years ago
After using the resizer on XP, the next mouse click on a chrome area is ignored
Steps to reproduce: 1. with a restored Firefox window, drag the corner re-sizer to re-size the window (make sure you are on the triangle re-sizer image and not on the very outside edge where you would be hitting the window frame and not the Firefox custom re-sizer image) 2. click and drag the title bar to move the window. Results: the drag fails and requires a second click because the focus was on the content area rather than the window decorations. This is pretty ugly and makes managing window size and position painful. It also has the effect of making the user feel a little bit stupid each time it happens. Tested with today's nightly build on Win XP.
cc'ing Jimm, overall this sounds like it should be a blocker, having to move focus to the window frame is a significant regression.
(In reply to comment #1) > cc'ing Jimm, overall this sounds like it should be a blocker, having to move > focus to the window frame is a significant regression. Hmm, this would be controlled by the drag window jsm code, or it's focus related maybe, not sure. You're clicking on xul content there, so I doubt it's a widget issue.
Window resizing is done on Windows by widget level code in nsWindow::BeginResizeDrag. Messages that occur: Mousedown on resizer: WM_LBUTTONDOWN WM_CAPTURECHANGED WM_MOUSEMOVE WM_SYSCOMMAND WM_GETMINMAXINFO WM_ENTERSIZEMOVE Mouseup on resizer: WM_CAPTURECHANGED WM_WINDOWPOSCHANGING WM_GETMINMAXINFO WM_EXITSIZEMOVE Note that no mouseup message is sent. Adding a response to WM_EXITSIZEMOVE which does the same thing as the response to WM_LBUTTONUP 'fixes' this bug. Note that this doesn't relate to the window titlebar. Anywhere in chrome is affected (as evidenced by theduplicate bug 633169) I would assume that this bug is caused because the lack of a mouseup event doesn't reset this state in the presshell/eventstatemanager. Something which resets these states should be happening as a response to the WM_EXITSIZEMOVE message, or probably more likely, the WM_CAPTURECHANGED message.
Component: General → Widget: Win32
Product: Firefox → Core
QA Contact: general → win32
Summary: using the custom resizer on XP breaks titlebar drag and drop a little → After using the resizer on XP, the next mouse click on a chrome area is ignored
How does this resizer work? (just want a quick idea on how it's positioned on top of content, and if it is a different window) It seems it's not releasing the mouse capture correctly. The reason that the window drag itself doesn't work is because the titlebar isn't draggable during mouse capture, but this is more obvious now with the report in bug 633169. Not quite the same but a similar condition we had to cover in the past was when the mouse moves between the client and non-client area. See http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsWindow.cpp#5085
On Windows, the resizing is done entirely by the operating system. Comment 3 lists the messages that are sent to the window during this process.
blocking2.0: ? → final+
Assignee: nobody → enndeakin
This patch listens for the WM_CAPTURECHANGED message. To reduce the regression possibility, it only does anything when sInInMouseCapture is false, as is the case for this bug, since I worry that clearing it always may affect some behaviour that should occur on mouseup.
Comment on attachment 512786 [details] [diff] [review] fix Olli, is this a suitable use of ClearGlobalActiveContent?
Attachment #512786 - Flags: review?(Olli.Pettay)
Comment on attachment 512786 [details] [diff] [review] fix Yes, if I understand WM_CAPTURECHANGED correctly.
Attachment #512786 - Flags: review?(Olli.Pettay) → review+
We call SetCapture in widget on mouse down and mouse up in DispatchMouseEvent, so this is going to get called on every mouse click in content. Is that acceptable? http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsWindow.cpp#3970
In which order do we get the WM_* events? Could we use WM_EXITSIZEMOVE and not WM_CAPTURECHANGED?
Hmm, though, there is the sIsInMouseCapture check. I assume that is set when mouse down starts capturing and is unset when mouse up happens
WM_CAPTURECHANGED is only fired on mouseup. At the point, sIsInMouseCapture is true, so nothing different happens. The only time I've found it sent on mousedown when sIsInMouseCapture is false is on the titlebar, although it occurs a few moments after the mousedown. I could change the patch to use WM_EXITSIZEMOVE instead if desired.
Wouldn't WM_EXITSIZEMOVE be less regression-risky? If so, better to use it.
Can we get an assessment of risk vs. reward? Touching events is scary.
<jimm> risk: seems low. it's isolated to when we have a mouse down on content and the window is being resized. reward: modest. it fixes a user interaction annoyance when you resize the window via the resizer on XP.
Comment on attachment 512808 [details] [diff] [review] use WM_EXITSIZEMOVE instead Decided against the risk of touching events at this point in the game. Risk aversion won out, here.
Attachment #512808 - Flags: approval2.0? → approval2.0-
We'll get this into the 5.0 (summer) release. People won't have to wait too long.
blocking2.0: final+ → -
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.