diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationBuildTasks/Microsoft.WinFx.props b/src/Microsoft.DotNet.Wpf/src/PresentationBuildTasks/Microsoft.WinFx.props index 59b680db8bbbe6006ae273981ae55a93c094765d..a73f7b23e5fd43ba20b91e1454bb75990f2d9c4c 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationBuildTasks/Microsoft.WinFx.props +++ b/src/Microsoft.DotNet.Wpf/src/PresentationBuildTasks/Microsoft.WinFx.props @@ -2,7 +2,7 @@ <_PresentationBuildTasksTfm Condition="'$(MSBuildRuntimeType)' == 'Core'">netcoreapp2.1 <_PresentationBuildTasksTfm Condition="'$(MSBuildRuntimeType)' != 'Core'">net472 - <_PresentationBuildTasksAssembly Condition="'$(_PresentationBuildTasksAssembly)'==''">$([System.IO.Path]::GetFullPath('$(MSBuildThisFileDirectory)..\tools\$(_PresentationBuildTasksTfm)\PresentationBuildTasks.dll')) + <_PresentationBuildTasksAssembly Condition="'$(_PresentationBuildTasksAssembly)'==''">$([MSBuild]::Unescape($([System.IO.Path]::GetFullPath('$(MSBuildThisFileDirectory)..\tools\$(_PresentationBuildTasksTfm)\PresentationBuildTasks.dll')))) diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/DefinitionBase.cs b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/DefinitionBase.cs index 001599594c75f349456552c99796a86a92200ad2..20c1f8ffe8a0d713372dad6cea086f7e5890865e 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/DefinitionBase.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/DefinitionBase.cs @@ -83,7 +83,7 @@ internal void OnEnterParentTree() { if (_sharedState == null) { - // start with getting SharedSizeGroup value. + // start with getting SharedSizeGroup value. // this property is NOT inhereted which should result in better overall perf. string sharedSizeGroupId = SharedSizeGroup; if (sharedSizeGroupId != null) @@ -167,8 +167,8 @@ internal static void OnUserSizePropertyChanged(DependencyObject d, DependencyPro parentGrid.Invalidate(); } else - { - parentGrid.InvalidateMeasure(); + { + parentGrid.InvalidateMeasure(); } } } @@ -411,6 +411,14 @@ internal double MinSizeForArrange } } + /// + /// Returns min size, never taking into account shared state. + /// + internal double RawMinSize + { + get { return _minSize; } + } + /// /// Offset. /// @@ -426,7 +434,7 @@ internal double FinalOffset internal GridLength UserSizeValueCache { get - { + { return (GridLength) GetValue( _isColumnDefinition ? ColumnDefinition.WidthProperty : @@ -442,8 +450,8 @@ internal double UserMinSizeValueCache get { return (double) GetValue( - _isColumnDefinition ? - ColumnDefinition.MinWidthProperty : + _isColumnDefinition ? + ColumnDefinition.MinWidthProperty : RowDefinition.MinHeightProperty); } } @@ -504,7 +512,7 @@ private bool CheckFlagsAnd(Flags flags) private static void OnSharedSizeGroupPropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e) { DefinitionBase definition = (DefinitionBase) d; - + if (definition.InParentLogicalTree) { string sharedSizeGroupId = (string) e.NewValue; @@ -516,13 +524,13 @@ private static void OnSharedSizeGroupPropertyChanged(DependencyObject d, Depende definition._sharedState.RemoveMember(definition); definition._sharedState = null; } - + if ((definition._sharedState == null) && (sharedSizeGroupId != null)) { SharedSizeScope privateSharedSizeScope = definition.PrivateSharedSizeScope; if (privateSharedSizeScope != null) { - // if definition is not registered and both: shared size group id AND private shared scope + // if definition is not registered and both: shared size group id AND private shared scope // are available, then register definition. definition._sharedState = privateSharedSizeScope.EnsureSharedState(sharedSizeGroupId); definition._sharedState.AddMember(definition); @@ -604,7 +612,7 @@ private static void OnPrivateSharedSizeScopePropertyChanged(DependencyObject d, string sharedSizeGroup = definition.SharedSizeGroup; if (sharedSizeGroup != null) { - // if definition is not registered and both: shared size group id AND private shared scope + // if definition is not registered and both: shared size group id AND private shared scope // are available, then register definition. definition._sharedState = privateSharedSizeScope.EnsureSharedState(definition.SharedSizeGroup); definition._sharedState.AddMember(definition); @@ -670,7 +678,7 @@ private bool LayoutWasUpdated private double _offset; // offset of the DefinitionBase from left / top corner (assuming LTR case) private SharedSizeState _sharedState; // reference to shared state object this instance is registered with - + internal const bool ThisIsColumnDefinition = true; internal const bool ThisIsRowDefinition = false; @@ -872,7 +880,7 @@ private void OnLayoutUpdated(object sender, EventArgs e) // accumulate min size of all participating definitions for (int i = 0, count = _registry.Count; i < count; ++i) { - sharedMinSize = Math.Max(sharedMinSize, _registry[i].MinSize); + sharedMinSize = Math.Max(sharedMinSize, _registry[i]._minSize); } bool sharedMinSizeChanged = !DoubleUtil.AreClose(_minSize, sharedMinSize); @@ -882,31 +890,65 @@ private void OnLayoutUpdated(object sender, EventArgs e) { DefinitionBase definitionBase = _registry[i]; - if (sharedMinSizeChanged || definitionBase.LayoutWasUpdated) + // we'll set d.UseSharedMinimum to maintain the invariant: + // d.UseSharedMinimum iff d._minSize < this.MinSize + // i.e. iff d is not a "long-pole" definition. + // + // Measure/Arrange of d's Grid uses d._minSize for long-pole + // definitions, and max(d._minSize, shared size) for + // short-pole definitions. This distinction allows us to react + // to changes in "long-pole-ness" more efficiently and correctly, + // by avoiding remeasures when a long-pole definition changes. + bool useSharedMinimum = !DoubleUtil.AreClose(definitionBase._minSize, sharedMinSize); + + // before doing that, determine whether d's Grid needs to be remeasured. + // It's important _not_ to remeasure if the last measure is still + // valid, otherwise infinite loops are possible + bool measureIsValid; + if (!definitionBase.UseSharedMinimum) { - // if definition's min size is different, then need to re-measure - if (!DoubleUtil.AreClose(sharedMinSize, definitionBase.MinSize)) - { - Grid parentGrid = (Grid)definitionBase.Parent; - parentGrid.InvalidateMeasure(); - definitionBase.UseSharedMinimum = true; - } - else - { - definitionBase.UseSharedMinimum = false; - - // if measure is valid then also need to check arrange. - // Note: definitionBase.SizeCache is volatile but at this point - // it contains up-to-date final size - if (!DoubleUtil.AreClose(sharedMinSize, definitionBase.SizeCache)) - { - Grid parentGrid = (Grid)definitionBase.Parent; - parentGrid.InvalidateArrange(); - } - } + // d was a long-pole. measure is valid iff it's still a long-pole, + // since previous measure didn't use shared size. + measureIsValid = !useSharedMinimum; + } + else if (useSharedMinimum) + { + // d was a short-pole, and still is. measure is valid + // iff the shared size didn't change + measureIsValid = !sharedMinSizeChanged; + } + else + { + // d was a short-pole, but is now a long-pole. This can + // happen in several ways: + // a. d's minSize increased to or past the old shared size + // b. other long-pole definitions decreased, leaving + // d as the new winner + // In the former case, the measure is valid - it used + // d's new larger minSize. In the latter case, the + // measure is invalid - it used the old shared size, + // which is larger than d's (possibly changed) minSize + measureIsValid = (definitionBase.LayoutWasUpdated && + DoubleUtil.GreaterThanOrClose(definitionBase._minSize, this.MinSize)); + } - definitionBase.LayoutWasUpdated = false; + if (!measureIsValid) + { + Grid parentGrid = (Grid)definitionBase.Parent; + parentGrid.InvalidateMeasure(); + } + else if (!DoubleUtil.AreClose(sharedMinSize, definitionBase.SizeCache)) + { + // if measure is valid then also need to check arrange. + // Note: definitionBase.SizeCache is volatile but at this point + // it contains up-to-date final size + Grid parentGrid = (Grid)definitionBase.Parent; + parentGrid.InvalidateArrange(); } + + // now we can restore the invariant, and clear the layout flag + definitionBase.UseSharedMinimum = useSharedMinimum; + definitionBase.LayoutWasUpdated = false; } _minSize = sharedMinSize; @@ -916,7 +958,7 @@ private void OnLayoutUpdated(object sender, EventArgs e) _broadcastInvalidation = true; } - + private readonly SharedSizeScope _sharedSizeScope; // the scope this state belongs to private readonly string _sharedSizeGroupId; // Id of the shared size group this object is servicing private readonly List _registry; // registry of participating definitions diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/Grid.cs b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/Grid.cs index 3700b92d685aeda1dc75ecfad67095ad42247c95..c18d6230b8dace6c368c3f7ed61eb81913f7bf23 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/Grid.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/Grid.cs @@ -1178,11 +1178,11 @@ private double[] CacheMinSizes(int cellsHead, bool isRows) { if (isRows) { - minSizes[PrivateCells[i].RowIndex] = DefinitionsV[PrivateCells[i].RowIndex].MinSize; + minSizes[PrivateCells[i].RowIndex] = DefinitionsV[PrivateCells[i].RowIndex].RawMinSize; } else { - minSizes[PrivateCells[i].ColumnIndex] = DefinitionsU[PrivateCells[i].ColumnIndex].MinSize; + minSizes[PrivateCells[i].ColumnIndex] = DefinitionsU[PrivateCells[i].ColumnIndex].RawMinSize; } i = PrivateCells[i].Next; diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/ItemsControl.cs b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/ItemsControl.cs index 51d708d2296bffcea12924c71a65f4aaf476bf5a..fc05da1d0e865996fcac9387f5efe3f4efcc6f90 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/ItemsControl.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/ItemsControl.cs @@ -60,7 +60,7 @@ public ItemsControl() : base() static ItemsControl() { - // Define default style in code instead of in theme files. + // Define default style in code instead of in theme files. DefaultStyleKeyProperty.OverrideMetadata(typeof(ItemsControl), new FrameworkPropertyMetadata(typeof(ItemsControl))); _dType = DependencyObjectType.FromSystemTypeInternal(typeof(ItemsControl)); EventManager.RegisterClassHandler(typeof(ItemsControl), Keyboard.GotKeyboardFocusEvent, new KeyboardFocusChangedEventHandler(OnGotFocus)); @@ -3011,7 +3011,7 @@ private bool IsOnCurrentPage(FrameworkElement viewPort, FrameworkElement element Rect viewPortBounds = new Rect(new Point(), viewPort.RenderSize); Rect elementBounds = new Rect(new Point(), element.RenderSize); - elementBounds = element.TransformToAncestor(viewPort).TransformBounds(elementBounds); + elementBounds = CorrectCatastrophicCancellation(element.TransformToAncestor(viewPort)).TransformBounds(elementBounds); bool northSouth = (axis == FocusNavigationDirection.Up || axis == FocusNavigationDirection.Down); bool eastWest = (axis == FocusNavigationDirection.Left || axis == FocusNavigationDirection.Right); @@ -3078,6 +3078,46 @@ private bool IsOnCurrentPage(FrameworkElement viewPort, FrameworkElement element return ElementViewportPosition.None; } + // in large virtualized hierarchical lists (TreeView or grouping), the transform + // returned by element.TransformToAncestor(viewport) is vulnerable to catastrophic + // cancellation. If element is at the top of the viewport, but embedded in + // layers of the hierarchy, the contributions of the intermediate elements add + // up to a large positive number which should exactly cancel out the large + // negative offset of the viewport's direct child to produce net offset of 0.0. + // But floating-point drift while accumulating the intermediate offsets and + // catastrophic cancellation in the last step may produce a very small + // non-zero number instead (e.g. -0.0000000000006548). This can lead to + // infinite loops and incorrect decisions in layout. + // To mitigate this problem, replace near-zero offsets with zero. + private static GeneralTransform CorrectCatastrophicCancellation(GeneralTransform transform) + { + MatrixTransform matrixTransform = transform as MatrixTransform; + if (matrixTransform != null) + { + bool needNewTransform = false; + Matrix matrix = matrixTransform.Matrix; + + if (matrix.OffsetX != 0.0 && LayoutDoubleUtil.AreClose(matrix.OffsetX, 0.0)) + { + matrix.OffsetX = 0.0; + needNewTransform = true; + } + + if (matrix.OffsetY != 0.0 && LayoutDoubleUtil.AreClose(matrix.OffsetY, 0.0)) + { + matrix.OffsetY = 0.0; + needNewTransform = true; + } + + if (needNewTransform) + { + transform = new MatrixTransform(matrix); + } + } + + return transform; + } + private static bool ElementIntersectsViewport(Rect viewportRect, Rect elementRect) { if (viewportRect.IsEmpty || elementRect.IsEmpty) diff --git a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/VirtualizingStackPanel.cs b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/VirtualizingStackPanel.cs index 2ab2a9035f56641a1f8a70a7f4944b5b90b3e78d..269b64e1cf55e0565eddc6e5d45b1f14c693dc7d 100644 --- a/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/VirtualizingStackPanel.cs +++ b/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/VirtualizingStackPanel.cs @@ -1237,7 +1237,7 @@ private void ClearAnchorInformation(bool shouldAbort) // don't delve into non-VSP panels - the result is // inconsistent with FindScrollOffset, and causes - // infinite loops + // infinite loops innerPanel = innerPanel as VirtualizingStackPanel; if (innerPanel != null && innerPanel.IsVisible) @@ -2279,7 +2279,7 @@ private Size RealMeasureOverride(Size constraint) // final size (e.g. when content arrives via bindings // that don't activate until after layout), as it // causes a lot of extra measure passes, where "a lot" - // can be as bad as "infinite". + // can be as bad as "infinite". // // To avoid this waste, exclude a proportional part // of the normal viewport as well. @@ -2812,7 +2812,7 @@ private Size RealMeasureOverride(Size constraint) // If there were a collection changed between the BringIndexIntoView call // and the current Measure then it is possible that the item for the // _bringIntoViewContainer has been removed from the collection and so - // has the container. We need to guard against this scenario. + // has the container. We need to guard against this scenario. _bringIntoViewContainer = null; } else @@ -4609,7 +4609,7 @@ private void ClearIsScrollActive() // if the viewport is empty in the scrolling direction, force the // cache to be empty also. This avoids an infinite loop re- and - // de-virtualizing the last item + // de-virtualizing the last item if (IsViewportEmpty(isHorizontal, viewport)) { cacheLength = new VirtualizationCacheLength(0, 0); @@ -5550,7 +5550,7 @@ private void CoerceScrollingViewportOffset(ref Rect viewport, Size extent, bool bool areContainersUniformlySized, double uniformOrAverageContainerSize) { - if (firstContainer == null) + if (firstContainer == null || IsViewportEmpty(isHorizontal, viewport)) { return -1.0; // undefined if no children in view } @@ -6149,7 +6149,7 @@ private void CoerceScrollingViewportOffset(ref Rect viewport, Size extent, bool // into non-uniform mode just because the pixel heights are different, // and a good reason to avoid it: it requires looping through all // containers to compute an average size, which breaks third-party's - // attempts to do data virtualization + // attempts to do data virtualization bool ignoreContainerPixelSize = IsPixelBased || (IsScrolling && !hasVirtualizingChildren); if (isHorizontal) @@ -9526,7 +9526,7 @@ private void OnScrollChange() // // We do not want the cache measure pass to affect the visibility // of the scrollbars because this makes bad user experience and - // is also the source of scrolling bugs. + // is also the source of scrolling bugs. // stackPixelSize.Width = _scrollData._extent.Width; @@ -9641,7 +9641,7 @@ private void OnScrollChange() offsetForScrollViewerRemeasure.X = newOffset; // adjust the persisted viewports too, in case the next use - // occurs before a measure, e.g. adding item to Items + // occurs before a measure, e.g. adding item to Items _viewport.X = newOffset; _extendedViewport.X += delta; @@ -9666,7 +9666,7 @@ private void OnScrollChange() offsetForScrollViewerRemeasure.Y = newOffset; // adjust the persisted viewports too, in case the next use - // occurs before a measure, e.g. adding item to Items + // occurs before a measure, e.g. adding item to Items _viewport.Y = newOffset; _extendedViewport.Y += delta; diff --git a/src/Microsoft.DotNet.Wpf/src/System.Windows.Controls.Ribbon/Microsoft/Windows/Controls/Ribbon/RibbonHelper.cs b/src/Microsoft.DotNet.Wpf/src/System.Windows.Controls.Ribbon/Microsoft/Windows/Controls/Ribbon/RibbonHelper.cs index 8c6413b155ea0037942536f7748662855d9bef69..be37b88666aaff408112c256457d4c82b5da159f 100644 --- a/src/Microsoft.DotNet.Wpf/src/System.Windows.Controls.Ribbon/Microsoft/Windows/Controls/Ribbon/RibbonHelper.cs +++ b/src/Microsoft.DotNet.Wpf/src/System.Windows.Controls.Ribbon/Microsoft/Windows/Controls/Ribbon/RibbonHelper.cs @@ -1,7 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. - + #if RIBBON_IN_FRAMEWORK namespace System.Windows.Controls.Ribbon @@ -1059,7 +1059,11 @@ public static bool IsOurWindow(IntPtr hwnd, DependencyObject element) // popups when both active source and current captured is // null due to clicking some where else should be handled by // click through event handler. - ReacquireCapture(targetCapture, targetFocus); + if (!ReacquireCapture(targetCapture, targetFocus)) + { + // call the setter if we couldn't reacquire capture + setter(false); + } e.Handled = true; } else @@ -1078,7 +1082,11 @@ public static bool IsOurWindow(IntPtr hwnd, DependencyObject element) { // If a descendant of targetCapture is losing capture // then take capture on targetCapture - ReacquireCapture(targetCapture, targetFocus); + if (!ReacquireCapture(targetCapture, targetFocus)) + { + // call the setter if we couldn't reacquire capture + setter(false); + } e.Handled = true; } else if (!IsCaptureInSubtree(targetCapture)) @@ -1092,13 +1100,14 @@ public static bool IsOurWindow(IntPtr hwnd, DependencyObject element) } } - private static void ReacquireCapture(UIElement targetCapture, UIElement targetFocus) + private static bool ReacquireCapture(UIElement targetCapture, UIElement targetFocus) { - Mouse.Capture(targetCapture, CaptureMode.SubTree); - if (targetFocus != null && !targetFocus.IsKeyboardFocusWithin) + bool success = Mouse.Capture(targetCapture, CaptureMode.SubTree); + if (success && targetFocus != null && !targetFocus.IsKeyboardFocusWithin) { targetFocus.Focus(); } + return success; } public static bool IsMousePhysicallyOver(UIElement element) diff --git a/src/Microsoft.DotNet.Wpf/src/System.Windows.Controls.Ribbon/Microsoft/Windows/Controls/Ribbon/RibbonMenuButton.cs b/src/Microsoft.DotNet.Wpf/src/System.Windows.Controls.Ribbon/Microsoft/Windows/Controls/Ribbon/RibbonMenuButton.cs index cf738775beb53036101d1abd314971a1b13758cd..7633b510fc3760accbd371a968beca6584676999 100644 --- a/src/Microsoft.DotNet.Wpf/src/System.Windows.Controls.Ribbon/Microsoft/Windows/Controls/Ribbon/RibbonMenuButton.cs +++ b/src/Microsoft.DotNet.Wpf/src/System.Windows.Controls.Ribbon/Microsoft/Windows/Controls/Ribbon/RibbonMenuButton.cs @@ -1,7 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. - + #if RIBBON_IN_FRAMEWORK namespace System.Windows.Controls.Ribbon @@ -30,11 +30,11 @@ namespace Microsoft.Windows.Controls.Ribbon using Microsoft.Windows.Controls.Ribbon.Primitives; #endif using MS.Internal; - + #endregion /// - /// RibbonMenuButton is an ItemsControl which on clicking displays a Menu. Its Items could be either RibbonMenuItems, RibbonGallerys or Separators. + /// RibbonMenuButton is an ItemsControl which on clicking displays a Menu. Its Items could be either RibbonMenuItems, RibbonGallerys or Separators. /// [TemplatePart(Name = RibbonMenuButton.ResizeThumbTemplatePartName, Type = typeof(Thumb))] [TemplatePart(Name = RibbonMenuButton.ToggleButtonTemplatePartName, Type = typeof(RibbonToggleButton))] @@ -592,9 +592,9 @@ protected override void ClearContainerForItemOverride(DependencyObject element, { base.ClearContainerForItemOverride(element, item); - // RibbonComboBox containers are pre-generated. - // When dropdown is opened for the first time ever and ItemContainerGenerator - // is hooked up to ItemsPanel, existing containers are cleared, causing _galleryCount to be -ve. + // RibbonComboBox containers are pre-generated. + // When dropdown is opened for the first time ever and ItemContainerGenerator + // is hooked up to ItemsPanel, existing containers are cleared, causing _galleryCount to be -ve. // Hence the check for _galleryCount > 0 if (element is RibbonGallery && _galleryCount > 0) { @@ -672,7 +672,7 @@ protected override bool HandlesScrolling public override void OnApplyTemplate() { - // If a new template has just been generated then + // If a new template has just been generated then // be sure to clear any stale ItemsHost references if (InternalItemsHost != null && !this.IsAncestorOf(InternalItemsHost)) { @@ -824,7 +824,7 @@ internal void OnNavigationKeyDown(KeyEventArgs e) { if (itemNavigateFromCurrentFocused) { - // event could have bubbled up from MenuItem + // event could have bubbled up from MenuItem // when it could not navigate to the next item (for eg. gallery) handled = RibbonHelper.NavigateToNextMenuItemOrGallery(this, focusedIndex, BringIndexIntoView); } @@ -841,7 +841,7 @@ internal void OnNavigationKeyDown(KeyEventArgs e) { if (itemNavigateFromCurrentFocused) { - // event could have bubbled up from MenuItem + // event could have bubbled up from MenuItem // when it could not navigate to the previous item (for eg. gallery) handled = RibbonHelper.NavigateToPreviousMenuItemOrGallery(this, focusedIndex, BringIndexIntoView); } @@ -1015,7 +1015,7 @@ void OnPopupResize(object sender, DragDeltaEventArgs e) } /// - /// Called from UIA Peers. + /// Called from UIA Peers. /// /// /// @@ -1079,11 +1079,11 @@ private static void OnIsDropDownOpenChanged(DependencyObject d, DependencyProper internal virtual void OnIsDropDownOpenChanged(DependencyPropertyChangedEventArgs e) { // If the drop down is closed due to - // an action of context menu or if the - // ContextMenu for a parent (Ribbon) - // was opened by right clicking this - // RibbonMenuButton (RibbonApplicationMenu) - // then ContextMenuClosed event is never raised. + // an action of context menu or if the + // ContextMenu for a parent (Ribbon) + // was opened by right clicking this + // RibbonMenuButton (RibbonApplicationMenu) + // then ContextMenuClosed event is never raised. // Hence reset the flag. InContextMenu = false; @@ -1098,7 +1098,7 @@ internal virtual void OnIsDropDownOpenChanged(DependencyPropertyChangedEventArgs BaseOnIsKeyboardFocusWithin(); OnDropDownOpened(EventArgs.Empty); - // Clear local values + // Clear local values // so that when DropDown opens it shows in it original size and PlacementMode RibbonDropDownHelper.ClearLocalValues(_itemsPresenter, _popup); @@ -1110,7 +1110,7 @@ internal virtual void OnIsDropDownOpenChanged(DependencyPropertyChangedEventArgs } // IsDropDownPositionedAbove is updated asynchronously. - // As a result the resize thumb would change position and we could see a visual artifact of this change after Popup opens. + // As a result the resize thumb would change position and we could see a visual artifact of this change after Popup opens. Dispatcher.BeginInvoke(new DispatcherOperationCallback(UpdateDropDownPosition), DispatcherPriority.Loaded, new object[] { null }); } else @@ -1200,15 +1200,15 @@ private static void OnHasGalleryChanged(DependencyObject d, DependencyPropertyCh RibbonHelper.SetDropDownHeight(menuButton._itemsPresenter, (bool)e.NewValue, menuButton.DropDownHeight); } - // Note that when the HasGallery property changes we expect that the - // VerticalScrollBarVisibilityProperty for the primary _submenuScrollViewer - // that hosts galleries and/or menu items is updated. Even though this - // property is marked AffectsMeasure it doesn't exactly cause the additonal - // call to ScrollViewer.MeasureOverrider because HasGallery is typically - // updated during a Measure pass when PrepareContainerForItemOverride is - // called and thus the invalidation noops. To ensure that we call - // ScrollViewer.MeasureOverride another time after this property has been - // updated, we need to wait for the current Measure pass to subside and + // Note that when the HasGallery property changes we expect that the + // VerticalScrollBarVisibilityProperty for the primary _submenuScrollViewer + // that hosts galleries and/or menu items is updated. Even though this + // property is marked AffectsMeasure it doesn't exactly cause the additonal + // call to ScrollViewer.MeasureOverrider because HasGallery is typically + // updated during a Measure pass when PrepareContainerForItemOverride is + // called and thus the invalidation noops. To ensure that we call + // ScrollViewer.MeasureOverride another time after this property has been + // updated, we need to wait for the current Measure pass to subside and // then InvalidateMeasure on the _submenuScrollViewer. RibbonHelper.InvalidateScrollBarVisibility(menuButton._submenuScrollViewer); @@ -1281,7 +1281,7 @@ private object UpdateDropDownPosition(object arg) // Cache the screen bounds of the monitor in which the dropdown is opened _screenBounds = RibbonDropDownHelper.GetScreenBounds(_itemsPresenter, _popup); - + // Also cache the PopupRoot if opened for the first time if (_popupRoot == null && _itemsPresenter != null) { @@ -1394,7 +1394,7 @@ internal RibbonToggleButton PartToggleButton } /// - /// base exits MenuMode on any Mouse clicks. We want to prevent that. + /// base exits MenuMode on any Mouse clicks. We want to prevent that. /// /// protected override void HandleMouseButton(MouseButtonEventArgs e) @@ -1471,9 +1471,9 @@ protected override void OnPreviewMouseDown(MouseButtonEventArgs e) protected override void OnIsKeyboardFocusWithinChanged(DependencyPropertyChangedEventArgs e) { - // If IsKeyboardFocusWithin has become true, then do not + // If IsKeyboardFocusWithin has become true, then do not // call base.OnIsKeyboardFocusWithinChanged right away. - // Defer the bases call until DropDown gets opened or + // Defer the bases call until DropDown gets opened or // one of the descendants get focus. if (!IsKeyboardFocusWithin) { @@ -1513,7 +1513,10 @@ private void OnGotKeyboardFocusThunk(KeyboardFocusChangedEventArgs e) { // Call base.OnIsKeyboardFocusWithinChanged only if the new focus // is not a direct descendant of menu button. + // It's possible to get here when disabled, which can lead to a + // focus war resulting in StackOverflow. Don't start the war. if (e.OriginalSource != this && + this.IsEnabled && !TreeHelper.IsVisualAncestorOf(this, e.OriginalSource as DependencyObject)) { BaseOnIsKeyboardFocusWithin(); @@ -1528,13 +1531,19 @@ protected override void OnGotKeyboardFocus(KeyboardFocusChangedEventArgs e) if (ribbonCurrentSelection != null && IsDropDownOpen) { + // If the drop down is open and the ribbonCurrentSelection is valid + // but still popup doesnt have focus within, + // then focus the current selection. + // It's possible to get here when disabled, or when an app explicitly + // moves focus in a GotKeyboardFocus handler called earlier in the + // bubbling route. Either of these can lead to a + // focus war resulting in StackOverflow. Don't start the war UIElement popupChild = _popup.TryGetChild(); if (popupChild != null && + this.IsEnabled && + this.IsKeyboardFocusWithin && !popupChild.IsKeyboardFocusWithin) { - // If the drop down is open and the ribbonCurrentSelection is valid - // but still popup doesnt have focus within, - // then focus the current selection. ribbonCurrentSelection.Focus(); } }