Bug 1735071 Comment 13 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

I basically agree with Edgar, although removing SWP_NOACTIVATE for popups is confusing.  I can't explain why activating the popup changes anything about z-order, much less fixes this, but it does seem that activating the popup is not right.  I've instead changed that line from
```
    ::SetWindowPos(mWnd, owner ? 0 : HWND_TOPMOST, 0, 0, 0, 0, flags);	
```
to
```
    HWND owner = ::GetWindow(mWnd, GW_OWNER);
    if (owner) {
      // ePopupLevelTop popups should be above all else.  All other
      // types should be placed in front of their owner, without
      // changing the owner's z-level relative to other windows.
      if (PopupLevel() != ePopupLevelTop) {
        ::SetWindowPos(mWnd, owner, 0, 0, 0, 0, flags);
        ::SetWindowPos(owner, mWnd, 0, 0, 0, 0,
                       SWP_NOMOVE | SWP_NOSIZE | SWP_NOACTIVATE);
      } else {
        ::SetWindowPos(mWnd, HWND_TOP, 0, 0, 0, 0, flags);
      }
    } else {
      ::SetWindowPos(mWnd, HWND_TOPMOST, 0, 0, 0, 0, flags);
    }
```
which, since the popups are not `ePopupLevelTop`, moves them into place without moving the owner.  (The first SWP call moves the popup behind the owner but handles the `SWP_SHOWWINDOW` -- the second SWP moves the owner behind the popup, which was immediately behind the owner, so the owner's relative position wrt all other windows is unchanged.)

I've left the `ePopupLevelTop` case as `HWND_TOP`, although I think it probably should be `HWND_TOPMOST`, but that's not what we had.

Does this seem like a good approach?
I basically agree with Edgar, although removing SWP_NOACTIVATE for popups is confusing.  I can't explain why activating the popup changes anything about z-order, much less fixes this, but it does seem that activating the popup is not right.  I've instead changed that line from
```
    ::SetWindowPos(mWnd, owner ? 0 : HWND_TOPMOST, 0, 0, 0, 0, flags);	
```
to
```
    if (owner) {
      // ePopupLevelTop popups should be above all else.  All other
      // types should be placed in front of their owner, without
      // changing the owner's z-level relative to other windows.
      if (PopupLevel() != ePopupLevelTop) {
        ::SetWindowPos(mWnd, owner, 0, 0, 0, 0, flags);
        ::SetWindowPos(owner, mWnd, 0, 0, 0, 0,
                       SWP_NOMOVE | SWP_NOSIZE | SWP_NOACTIVATE);
      } else {
        ::SetWindowPos(mWnd, HWND_TOP, 0, 0, 0, 0, flags);
      }
    } else {
      ::SetWindowPos(mWnd, HWND_TOPMOST, 0, 0, 0, 0, flags);
    }
```
which, since the popups are not `ePopupLevelTop`, moves them into place without moving the owner.  (The first SWP call moves the popup behind the owner but handles the `SWP_SHOWWINDOW` -- the second SWP moves the owner behind the popup, which was immediately behind the owner, so the owner's relative position wrt all other windows is unchanged.)

I've left the `ePopupLevelTop` case as `HWND_TOP`, although I think it probably should be `HWND_TOPMOST`, but that's not what we had.

Does this seem like a good approach?

Back to Bug 1735071 Comment 13