From 4d4d3470a8c280e71ff7357727203d74b35a8554 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 4 Jun 2024 16:24:44 +0000 Subject: [PATCH] Bug 1892835 - Remove delayed NCACTIVATE hack. r=masayuki,win-reviewers,rkraesig See comment in the bug. This was needed for bogus mouse drivers back in the XP/Vista era. However Win10/11 sends wheel messages to the window under the cursor instead of focused window, so these kinds of issues shouldn't arise there. Try to remove this legacy hack. Differential Revision: https://phabricator.services.mozilla.com/D210302 --- widget/windows/WinMessages.h | 3 - widget/windows/nsWindow.cpp | 135 +++++------------------------------ widget/windows/nsWindow.h | 3 +- 3 files changed, 19 insertions(+), 122 deletions(-) diff --git a/widget/windows/WinMessages.h b/widget/windows/WinMessages.h index 8c90460e8e6a..9d8e596ae0a6 100644 --- a/widget/windows/WinMessages.h +++ b/widget/windows/WinMessages.h @@ -25,9 +25,6 @@ #define MOZ_WM_HSCROLL (WM_APP + 0x0313) #define MOZ_WM_MOUSEWHEEL_FIRST MOZ_WM_MOUSEVWHEEL #define MOZ_WM_MOUSEWHEEL_LAST MOZ_WM_HSCROLL -// If a popup window is being activated, we try to reactivate the previous -// window with this message. -#define MOZ_WM_REACTIVATE (WM_APP + 0x0314) // If TSFTextStore needs to notify TSF/TIP of layout change later, this // message is posted. #define MOZ_WM_NOTIY_TSF_OF_LAYOUT_CHANGE (WM_APP + 0x0315) diff --git a/widget/windows/nsWindow.cpp b/widget/windows/nsWindow.cpp index ac393e8b0932..c75bb2d2fadd 100644 --- a/widget/windows/nsWindow.cpp +++ b/widget/windows/nsWindow.cpp @@ -7724,23 +7724,6 @@ bool nsWindow::GetPopupsToRollup(nsIRollupListener* aRollupListener, return true; } -// static -bool nsWindow::NeedsToHandleNCActivateDelayed(HWND aWnd) { - // While popup is open, popup window might be activated by other application. - // At this time, we need to take back focus to the previous window but it - // causes flickering its nonclient area because WM_NCACTIVATE comes before - // WM_ACTIVATE and we cannot know which window will take focus at receiving - // WM_NCACTIVATE. Therefore, we need a hack for preventing the flickerling. - // - // If non-popup window receives WM_NCACTIVATE at deactivating, default - // wndproc shouldn't handle it as deactivating. Instead, at receiving - // WM_ACTIVIATE after that, WM_NCACTIVATE should be sent again manually. - // This returns true if the window needs to handle WM_NCACTIVATE later. - - nsWindow* window = WinUtils::GetNSWindowPtr(aWnd); - return window && !window->IsPopup(); -} - static bool IsTouchSupportEnabled(HWND aWnd) { nsWindow* topWindow = WinUtils::GetNSWindowPtr(WinUtils::GetTopLevelHWND(aWnd, true)); @@ -7794,8 +7777,6 @@ bool nsWindow::DealWithPopups(HWND aWnd, UINT aMessage, WPARAM aWParam, return false; } - static bool sSendingNCACTIVATE = false; - static bool sPendingNCACTIVATE = false; uint32_t popupsToRollup = UINT32_MAX; bool consumeRollupEvent = false; @@ -7888,110 +7869,30 @@ bool nsWindow::DealWithPopups(HWND aWnd, UINT aMessage, WPARAM aWParam, break; case WM_ACTIVATE: { + // This marker should be useless nowadays, but kept just for safety, see + // the discussion in D210302. See also bug 1842170. WndProcUrgentInvocation::Marker _marker; - // NOTE: Don't handle WA_INACTIVE for preventing popup taking focus - // because we cannot distinguish it's caused by mouse or not. - if (LOWORD(aWParam) == WA_ACTIVE && aLParam) { - nsWindow* window = WinUtils::GetNSWindowPtr(aWnd); - if (window && (window->IsPopup() || window->mIsAlert)) { - // Cancel notifying widget listeners of deactivating the previous - // active window (see WM_KILLFOCUS case in ProcessMessage()). - sJustGotDeactivate = false; - // Reactivate the window later. - ::PostMessageW(aWnd, MOZ_WM_REACTIVATE, aWParam, aLParam); - return true; - } - // Don't rollup the popup when focus moves back to the parent window - // from a popup because such case is caused by strange mouse drivers. - nsWindow* prevWindow = - WinUtils::GetNSWindowPtr(reinterpret_cast(aLParam)); - if (prevWindow && prevWindow->IsPopup()) { - // Consume this message here since previous window must not have - // been inactivated since we've already stopped accepting the - // inactivation below. - return true; - } - } else if (LOWORD(aWParam) == WA_INACTIVE) { - nsWindow* activeWindow = - WinUtils::GetNSWindowPtr(reinterpret_cast(aLParam)); - if (sPendingNCACTIVATE && NeedsToHandleNCActivateDelayed(aWnd)) { - // If focus moves to non-popup widget or focusable popup, the window - // needs to update its nonclient area. - if (!activeWindow || !activeWindow->IsPopup()) { - sSendingNCACTIVATE = true; - ::SendMessageW(aWnd, WM_NCACTIVATE, false, 0); - sSendingNCACTIVATE = false; - } - sPendingNCACTIVATE = false; - } - // If focus moves from/to popup, we don't need to rollup the popup - // because such case is caused by strange mouse drivers. And in - // such case, we should consume the message here since we need to - // hide this odd focus move from our content. (If we didn't consume - // the message here, ProcessMessage() will notify widget listener of - // inactivation and that causes unnecessary reflow for supporting - // -moz-window-inactive pseudo class. - if (activeWindow) { - if (activeWindow->IsPopup()) { - return true; - } - nsWindow* deactiveWindow = WinUtils::GetNSWindowPtr(aWnd); - if (deactiveWindow && deactiveWindow->IsPopup()) { - return true; - } - } - } else if (LOWORD(aWParam) == WA_CLICKACTIVE) { - // If the WM_ACTIVATE message is caused by a click in a popup, - // we should not rollup any popups. - nsWindow* window = WinUtils::GetNSWindowPtr(aWnd); - if ((window && window->IsPopup()) || - !GetPopupsToRollup(rollupListener, &popupsToRollup)) { - return false; - } + nsWindow* window = WinUtils::GetNSWindowPtr(aWnd); + nsWindow* prevWindow = + WinUtils::GetNSWindowPtr(reinterpret_cast(aLParam)); + // Don't rollup popups for WM_ACTIVATE from/to a popup. + // When we click on a popup (WA_CLICKACTIVE) we don't want to do it. + // WA_ACTIVE/WA_INACTIVE shouldn't really happen, but some old + // pre-windows-10 drivers used to do this, see bug 953146. + // It might be the case that is no longer needed tho, and we can move + // this to the WA_CLICKACTIVE condition. + if ((window && window->IsPopup()) || + (prevWindow && prevWindow->IsPopup())) { + return false; + } + if (LOWORD(aWParam) == WA_CLICKACTIVE && + !GetPopupsToRollup(rollupListener, &popupsToRollup)) { + return false; } allowAnimations = nsIRollupListener::AllowAnimations::No; } break; - case MOZ_WM_REACTIVATE: - // The previous active window should take back focus. - if (::IsWindow(reinterpret_cast(aLParam))) { - // FYI: Even without this API call, you see expected result (e.g., the - // owner window of the popup keeps active without flickering - // the non-client area). And also this causes initializing - // TSF and it causes using CPU time a lot. However, even if we - // consume WM_ACTIVE messages, native focus change has already - // been occurred. I.e., a popup window is active now. Therefore, - // you'll see some odd behavior if we don't reactivate the owner - // window here. For example, if you do: - // 1. Turn wheel on a bookmark panel. - // 2. Turn wheel on another window. - // then, you'll see that the another window becomes active but the - // owner window of the bookmark panel looks still active and the - // bookmark panel keeps open. The reason is that the first wheel - // operation gives focus to the bookmark panel. Therefore, when - // the next operation gives focus to the another window, previous - // focus window is the bookmark panel (i.e., a popup window). - // So, in this case, our hack around here prevents to inactivate - // the owner window and roll up the bookmark panel. - ::SetForegroundWindow(reinterpret_cast(aLParam)); - } - return true; - - case WM_NCACTIVATE: - if (!aWParam && !sSendingNCACTIVATE && - NeedsToHandleNCActivateDelayed(aWnd)) { - // Don't just consume WM_NCACTIVATE. It doesn't handle only the - // nonclient area state change. - ::DefWindowProcW(aWnd, aMessage, TRUE, aLParam); - // Accept the deactivating because it's necessary to receive following - // WM_ACTIVATE. - *aResult = TRUE; - sPendingNCACTIVATE = true; - return true; - } - return false; - case WM_MOUSEACTIVATE: if (!EventIsInsideWindow(popupWindow) && GetPopupsToRollup(rollupListener, &popupsToRollup)) { diff --git a/widget/windows/nsWindow.h b/widget/windows/nsWindow.h index 9c4b4417f813..1cca23b36992 100644 --- a/widget/windows/nsWindow.h +++ b/widget/windows/nsWindow.h @@ -598,7 +598,6 @@ class nsWindow final : public nsBaseWidget { static bool GetPopupsToRollup( nsIRollupListener* aRollupListener, uint32_t* aPopupsToRollup, mozilla::Maybe aEventPoint = mozilla::Nothing()); - static bool NeedsToHandleNCActivateDelayed(HWND aWnd); static bool DealWithPopups(HWND inWnd, UINT inMsg, WPARAM inWParam, LPARAM inLParam, LRESULT* outResult); @@ -869,7 +868,7 @@ class nsWindow final : public nsBaseWidget { class MOZ_STACK_CLASS ContextMenuPreventer final { public: explicit ContextMenuPreventer(nsWindow* aWindow) - : mWindow(aWindow), mNeedsToPreventContextMenu(false){}; + : mWindow(aWindow), mNeedsToPreventContextMenu(false) {}; ~ContextMenuPreventer() { mWindow->mNeedsToPreventContextMenu = mNeedsToPreventContextMenu; }