提交 0437c64a 编写于 作者: V Vatsan Madhavan

As part of a previous fix (that shipped originally as part of .NET 4.8 in), a...

As part of a previous fix (that shipped originally as part of .NET 4.8 in), a change was made to `Popup` that involved the destruction and recreation of the underlying `HWND`. This was done to ensure that the `HWND` was always created with the correct monitor (~=DPI) affinity.

In order to acheive this, `DestroyWindow()` and `BuildWindow()` - private methods in `Popup` - were used.

We are finding that `DestroyWindow()` has side-effects that can lead to incorrect behavior. The incorrect beahvior works as follows:

  - The call into `Popup.CreateWindow` is usually a consequence `Popup.IsOpenChanged` (`false -> true`)
  - Within `Popup.CreateWindow`, `DestroyWindow()` is called (when high-dpi mode is detected).
  - `DestroyWindow()` destroys the underlying `HWND`, releases the capture, raises the *OnClosed* event and clears the placement-target.
    - At the end of `DestroyWindow()`, we get `IsOpen == false`.
  - After `DestroyWindow()`, we call into `BuildWindow()` and `CreateNewPopupRoot()` etc., which go on to build the `Popup` again (and also instnatiate a new `HWND`).
    - Unfortunately, there is no mechanism here for resetting `IsOpen` back to `true` (without also leading to an undesirable infinite-recursion that calls back into `CreateWindow`).

If these calls to show the context-menu arise from `ContextMenu.OnIsOpenChanged`, and flow through `ContextMenu.HookupParentPopup -> Popup.CreateRootPopup`, then `IsOpen` gets reset (there is a direct call into `SetBinding(IsOpenProperty)`, in `Popup.CreateRootPopupInternal`) and the popup is shown correctly. Until then, the context-menu is "stuck" not being able to be shown.

The solution is to stop using `DestroyWindow()` as-is, which does more than what we need for it to accomplish. Our original intent in calling `DestroyWindow()` was simply destroy and recreate the `HWND`. This fix refactors `DestroyWindow()` to suit this need and uses the newly introduced `DestroyWindowImpl()` to destroy the `HWND`, and then recreate just that. The rest of the state is retained intact, and the `Popup/ContextMenu` continues to function well as before.
上级 86c905bc
......@@ -1510,7 +1510,7 @@ private void CreateWindow(bool asyncCall)
// cascading failure.
if (PopupInitialPlacementHelper.IsPerMonitorDpiScalingActive)
{
DestroyWindow();
DestroyWindowImpl();
_positionInfo = null;
makeNewWindow = true;
}
......@@ -1601,18 +1601,40 @@ private void BuildWindow(Visual targetVisual)
_secHelper.BuildWindow(origin.x, origin.y, targetVisual, IsTransparent, PopupFilterMessage, OnWindowResize, OnDpiChanged);
}
private void DestroyWindow()
/// <summary>
/// Destroys the underlying window (HWND) if it is alive
/// </summary>
/// <returns>true if the window was destroyed, otherwise false</returns>
private bool DestroyWindowImpl()
{
if (_secHelper.IsWindowAlive())
{
_secHelper.DestroyWindow(PopupFilterMessage, OnWindowResize, OnDpiChanged);
ReleasePopupCapture();
return true;
}
// Raise closed event after popup has actually closed
OnClosed(EventArgs.Empty);
return false;
}
// When closing, clear the placement target registration
UpdatePlacementTargetRegistration(PlacementTarget, null);
/// <summary>
/// Destroys the window, and does additional book-keeping
/// like releasing the capture, raising Closed event, and
/// clearing placement-target registration
/// </summary>
private void DestroyWindow()
{
if (_secHelper.IsWindowAlive())
{
if (DestroyWindowImpl())
{
ReleasePopupCapture();
// Raise closed event after popup has actually closed
OnClosed(EventArgs.Empty);
// When closing, clear the placement target registration
UpdatePlacementTargetRegistration(PlacementTarget, null);
}
}
}
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册