Bug 1399126 - Make nsWindow for Windows not notify widget listener of activated/inactivated if active window is changed from/to popup window r=jimm
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 10 Jul 2018 21:24:06 +0900
changeset 426463 9c66627fee188569cc6509ac4f2282f1edf68fd9
parent 426462 a4cb46dfbdd76db6fe83f0f6ee8f560642bb9b85
child 426464 1a9c6868329019bd23b15c4eafb44cc00edf225e
push id105231
push userebalazs@mozilla.com
push dateFri, 13 Jul 2018 09:00:11 +0000
treeherdermozilla-inbound@1e71794ed2ef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimm
bugs1399126, 953146
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
@@ -8054,56 +8054,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