From f56dca9254554d430558872be24affc47a314e9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 12 Feb 2024 19:45:48 +0000 Subject: [PATCH] Bug 1878037 - Fix some regressions on alert windows. r=saschanaz,win-reviewers,rkraesig Bug 1870512 removed top level windows with WindowType::Popup, instead replacing it with an "alert" feature for the only use of them that we had. Turns out that on Windows they need a bit more special handling than I thought, so this patch uses the alert boolean to fix two regressions (that alerts now create taskbar icons, and that they steal focus). Remove mIsForMenuPopupFrame since that is now a synonym with WindowType::Popup. Remove also the dialog vs. toplevel difference of using WS_EX_DLGMODALFRAME, since that doesn't seem to have an effect at all on windows 10+. Differential Revision: https://phabricator.services.mozilla.com/D200508 --- layout/xul/nsMenuPopupFrame.cpp | 1 - widget/InitData.h | 1 - widget/windows/nsWindow.cpp | 51 +++++++++++++++-------------- widget/windows/nsWindow.h | 9 +++-- xpfe/appshell/nsAppShellService.cpp | 8 ++++- 5 files changed, 40 insertions(+), 30 deletions(-) diff --git a/layout/xul/nsMenuPopupFrame.cpp b/layout/xul/nsMenuPopupFrame.cpp index 793779bb430b..b41a66670782 100644 --- a/layout/xul/nsMenuPopupFrame.cpp +++ b/layout/xul/nsMenuPopupFrame.cpp @@ -268,7 +268,6 @@ nsresult nsMenuPopupFrame::CreateWidgetForView(nsView* aView) { widget::InitData widgetData; widgetData.mWindowType = widget::WindowType::Popup; widgetData.mBorderStyle = widget::BorderStyle::Default; - widgetData.mForMenupopupFrame = true; widgetData.mClipSiblings = true; widgetData.mPopupHint = mPopupType; widgetData.mNoAutoHide = IsNoAutoHide(); diff --git a/widget/InitData.h b/widget/InitData.h index 6f71265df134..1498feed9aef 100644 --- a/widget/InitData.h +++ b/widget/InitData.h @@ -91,7 +91,6 @@ struct InitData { // when painting exclude area occupied by child windows and sibling windows bool mClipChildren = false; bool mClipSiblings = false; - bool mForMenupopupFrame = false; bool mRTL = false; bool mNoAutoHide = false; // true for noautohide panels bool mIsDragPopup = false; // true for drag feedback panels diff --git a/widget/windows/nsWindow.cpp b/widget/windows/nsWindow.cpp index 04dc18b8bf78..e7a242c147a8 100644 --- a/widget/windows/nsWindow.cpp +++ b/widget/windows/nsWindow.cpp @@ -868,9 +868,9 @@ nsresult nsWindow::Create(nsIWidget* aParent, nsNativeWidget aNativeParent, } mIsRTL = aInitData->mRTL; - mForMenupopupFrame = aInitData->mForMenupopupFrame; mOpeningAnimationSuppressed = aInitData->mIsAnimationSuppressed; mAlwaysOnTop = aInitData->mAlwaysOnTop; + mIsAlert = aInitData->mIsAlert; mResizable = aInitData->mResizable; DWORD style = WindowStyle(); @@ -895,7 +895,7 @@ nsresult nsWindow::Create(nsIWidget* aParent, nsNativeWidget aNativeParent, } } - const wchar_t* className = ChooseWindowClass(mWindowType, mForMenupopupFrame); + const wchar_t* className = ChooseWindowClass(mWindowType); // Take specific actions when creating the first top-level window static bool sFirstTopLevelWindowCreated = false; @@ -1220,20 +1220,15 @@ const wchar_t* nsWindow::RegisterWindowClass(const wchar_t* aClassName, static LPWSTR const gStockApplicationIcon = MAKEINTRESOURCEW(32512); /* static */ -const wchar_t* nsWindow::ChooseWindowClass(WindowType aWindowType, - bool aForMenupopupFrame) { - MOZ_ASSERT_IF(aForMenupopupFrame, aWindowType == WindowType::Popup); +const wchar_t* nsWindow::ChooseWindowClass(WindowType aWindowType) { switch (aWindowType) { case WindowType::Invisible: return RegisterWindowClass(kClassNameHidden, 0, gStockApplicationIcon); case WindowType::Dialog: return RegisterWindowClass(kClassNameDialog, 0, 0); case WindowType::Popup: - if (aForMenupopupFrame) { - return RegisterWindowClass(kClassNameDropShadow, CS_DROPSHADOW, - gStockApplicationIcon); - } - [[fallthrough]]; + return RegisterWindowClass(kClassNameDropShadow, CS_DROPSHADOW, + gStockApplicationIcon); default: return RegisterWindowClass(GetMainWindowClass(), 0, gStockApplicationIcon); @@ -1341,22 +1336,27 @@ DWORD nsWindow::WindowExStyle() { case WindowType::Child: return 0; - case WindowType::Dialog: - return WS_EX_WINDOWEDGE | WS_EX_DLGMODALFRAME; - case WindowType::Popup: { DWORD extendedStyle = WS_EX_TOOLWINDOW; - if (mPopupLevel == PopupLevel::Top) extendedStyle |= WS_EX_TOPMOST; + if (mPopupLevel == PopupLevel::Top) { + extendedStyle |= WS_EX_TOPMOST; + } return extendedStyle; } - default: - NS_ERROR("unknown border style"); - [[fallthrough]]; + case WindowType::Sheet: + MOZ_FALLTHROUGH_ASSERT("Sheets are macOS specific"); + case WindowType::Dialog: case WindowType::TopLevel: case WindowType::Invisible: - return WS_EX_WINDOWEDGE; + break; } + if (mIsAlert) { + MOZ_ASSERT(mWindowType == WindowType::Dialog, + "Expect alert windows to have type=dialog"); + return WS_EX_TOOLWINDOW; + } + return WS_EX_WINDOWEDGE; } /************************************************************** @@ -1575,9 +1575,8 @@ void nsWindow::Show(bool bState) { #endif // defined(ACCESSIBILITY) } - if (mForMenupopupFrame) { - MOZ_ASSERT(ChooseWindowClass(mWindowType, mForMenupopupFrame) == - kClassNameDropShadow); + if (mWindowType == WindowType::Popup) { + MOZ_ASSERT(ChooseWindowClass(mWindowType) == kClassNameDropShadow); const bool shouldUseDropShadow = mTransparencyMode != TransparencyMode::Transparent; @@ -1644,8 +1643,12 @@ void nsWindow::Show(bool bState) { } } else { DWORD flags = SWP_NOSIZE | SWP_NOMOVE | SWP_SHOWWINDOW; - if (wasVisible) flags |= SWP_NOZORDER; - if (mAlwaysOnTop) flags |= SWP_NOACTIVATE; + if (wasVisible) { + flags |= SWP_NOZORDER; + } + if (mAlwaysOnTop || mIsAlert) { + flags |= SWP_NOACTIVATE; + } if (mWindowType == WindowType::Popup) { // ensure popups are the topmost of the TOPMOST @@ -7858,7 +7861,7 @@ bool nsWindow::DealWithPopups(HWND aWnd, UINT aMessage, WPARAM aWParam, // 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()) { + if (window && (window->IsPopup() || window->mIsAlert)) { // Cancel notifying widget listeners of deactivating the previous // active window (see WM_KILLFOCUS case in ProcessMessage()). sJustGotDeactivate = false; diff --git a/widget/windows/nsWindow.h b/widget/windows/nsWindow.h index ed318a1d7b6e..f0994e9d348a 100644 --- a/widget/windows/nsWindow.h +++ b/widget/windows/nsWindow.h @@ -593,7 +593,7 @@ class nsWindow final : public nsBaseWidget { DWORD WindowStyle(); DWORD WindowExStyle(); - static const wchar_t* ChooseWindowClass(WindowType, bool aForMenupopupFrame); + static const wchar_t* ChooseWindowClass(WindowType); // This method registers the given window class, and returns the class name. static const wchar_t* RegisterWindowClass(const wchar_t* aClassName, UINT aExtraStyle, LPWSTR aIconID); @@ -754,7 +754,10 @@ class nsWindow final : public nsBaseWidget { bool mIsEarlyBlankWindow = false; bool mIsShowingPreXULSkeletonUI = false; bool mResizable = false; - bool mForMenupopupFrame = false; + // Whether we're an alert window. Alert windows don't have taskbar icons and + // don't steal focus from other windows when opened. They're also expected to + // be of type WindowType::Dialog. + bool mIsAlert = false; bool mIsPerformingDwmFlushHack = false; bool mDraggingWindowWithMouse = false; DWORD_PTR mOldStyle = 0; @@ -835,7 +838,7 @@ class nsWindow final : public nsBaseWidget { // Whether we're in the process of sending a WM_SETTEXT ourselves bool mSendingSetText = false; - // Whether we we're created as a child window (aka ChildWindow) or not. + // Whether we were created as a child window (aka ChildWindow) or not. bool mIsChildWindow : 1; int32_t mCachedHitTestResult = 0; diff --git a/xpfe/appshell/nsAppShellService.cpp b/xpfe/appshell/nsAppShellService.cpp index 55591f5abf52..1ac989aaa326 100644 --- a/xpfe/appshell/nsAppShellService.cpp +++ b/xpfe/appshell/nsAppShellService.cpp @@ -571,7 +571,13 @@ nsresult nsAppShellService::JustCreateTopWindow( } #endif - widgetInitData.mIsAlert = !!(aChromeMask & nsIWebBrowserChrome::CHROME_ALERT); + // alert=yes is expected to be used along with dialogs, not other window + // types. + MOZ_ASSERT_IF(aChromeMask & nsIWebBrowserChrome::CHROME_ALERT, + widgetInitData.mWindowType == widget::WindowType::Dialog); + widgetInitData.mIsAlert = + !!(aChromeMask & nsIWebBrowserChrome::CHROME_ALERT) && + widgetInitData.mWindowType == widget::WindowType::Dialog; #ifdef XP_MACOSX // Mac OS X sheet support