Bug 1399126 - Make nsWindow for Windows not notify widget listener of activated/inactivated if active window is changed from/to popup window r?jimm draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 10 Jul 2018 21:24:06 +0900
changeset 816382 0e6f94712de76c7f8fb111665f252c07f4dac7f9
parent 815997 056ef748448155db28be3a957538a307b06b88ab
push id115824
push usermasayuki@d-toybox.com
push dateWed, 11 Jul 2018 04:24:35 +0000
reviewersjimm
bugs1399126, 953146
milestone63.0a1
Bug 1399126 - Make nsWindow for Windows not notify widget listener of activated/inactivated if active window is changed from/to popup window r?jimm Some odd mouse drivers try to activate a window which the mouse driver wants to scroll its content (such window is typically under the mouse cursor when mouse wheel is turned). However, this is illegal behavior and such odd mouse drivers try to activate our popup windows which won't be activated without such apps. We prevented this odd focus behavior with fixing of bug 953146. However, it did NOT stop notifying widget listener of activating nor inactivating the windows. Therefore, that caused a lot of reflow for supporting -moz-window-inactive pseudo class. This patch makes nsWindow::DealWithPopups() consume WM_ACTIVATE message before nsWindow::ProcessMessage() because nsWindow::ProcessMessage() notifies widget listener of activating and inactivating window even when focus move from and to our popup window. So, in other words, this patch stops notifying widget listener of activating and inactivating window when focus moves from/to a popup window. MozReview-Commit-ID: 2dyq07zHZKp
widget/windows/nsWindow.cpp
--- a/widget/windows/nsWindow.cpp
+++ b/widget/windows/nsWindow.cpp
@@ -8045,56 +8045,82 @@ nsWindow::DealWithPopups(HWND aWnd, UINT
           ::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<HWND>(aLParam));
         if (prevWindow && prevWindow->IsPopup()) {
-          return false;
+          // 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<HWND>(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.
+        // 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 false;
+            return true;
           }
           nsWindow* deactiveWindow = WinUtils::GetNSWindowPtr(aWnd);
           if (deactiveWindow && deactiveWindow->IsPopup()) {
-            return false;
+            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;
         }
       }
       break;
 
     case MOZ_WM_REACTIVATE:
       // The previous active window should take back focus.
       if (::IsWindow(reinterpret_cast<HWND>(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<HWND>(aLParam));
       }
       return true;
 
     case WM_NCACTIVATE:
       if (!aWParam && !sSendingNCACTIVATE &&
           NeedsToHandleNCActivateDelayed(aWnd)) {
         // Don't just consume WM_NCACTIVATE. It doesn't handle only the