diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f7bca787..d82f29542 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - [SIL.Core] Added PathUtilities.ParentDirectories extension method. - [SIL.Core] Added FileLocationUtilities.DistFilesFolderPath property. - [SIL.Core.Clearshare] Added `MetadataCore.RunUnderTagLibLock(Action)` and `RunUnderTagLibLock(Func)` so callers that use TagLib directly can serialize that access against ClearShare's own metadata reading and writing. +- [SIL.Windows.Forms] Added `SettingsProtectionHelper.SetSettingsProtection` overloads taking a `keepHidden` parameter, so a `Control` or `ToolStripItem` can be marked always-hidden: it stays hidden even while Ctrl+Shift is held to reveal the other protected components. The existing two-parameter overloads are unchanged. ### Fixed @@ -57,6 +58,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - [SIL.Archiving] Fixed ArchiveAccessProtocol.GetDocumentationUri failing to create a missing documentation file because the resource lookup stripped the file extension and no longer matched the embedded resource name. - [SIL.Windows.Forms.Archiving] Fixed formatting of message in ArchivingDlg so that the name of the auxiliary archive upload program (e.g., "RAMP") is displayed. - [SIL.Windows.Forms] Fixed SettingsLauncherButton never disposing the SettingsProtectionHelper it creates, which left an enabled timer running after the button was disposed. Also removed the button's own unused visibility timer, which had no handler but was posting timer messages for the life of the control. +- [SIL.Windows.Forms] Fixed `SettingsProtectionHelper.Dispose` so that it only touches managed resources when disposing, is safe to call more than once, and no longer disposes the Ctrl+Shift timer explicitly (the timer is owned by the component container that disposes it). ### Changed diff --git a/SIL.Windows.Forms.Tests/SettingsProtection/SettingsProtectionHelperTests.cs b/SIL.Windows.Forms.Tests/SettingsProtection/SettingsProtectionHelperTests.cs new file mode 100644 index 000000000..8c10f6d92 --- /dev/null +++ b/SIL.Windows.Forms.Tests/SettingsProtection/SettingsProtectionHelperTests.cs @@ -0,0 +1,198 @@ +using System.ComponentModel; +using System.Reflection; +using System.Windows.Forms; +using NUnit.Framework; +using SIL.Windows.Forms.SettingProtection; + +namespace SIL.Windows.Forms.Tests.SettingsProtection +{ + [TestFixture] + [Apartment(System.Threading.ApartmentState.STA)] + public class SettingsProtectionHelperTests + { + private SettingsProtectionHelper _helper; + private bool _savedNormallyHidden; + + [SetUp] + public void SetUp() + { + _savedNormallyHidden = SettingsProtectionSingleton.Settings.NormallyHidden; + _helper = new SettingsProtectionHelper(null); + } + + [TearDown] + public void TearDown() + { + SettingsProtectionSingleton.Settings.NormallyHidden = _savedNormallyHidden; + _helper.Dispose(); + } + + private static void CallUpdateDisplay(SettingsProtectionHelper helper) + { + var method = typeof(SettingsProtectionHelper).GetMethod("UpdateDisplay", + BindingFlags.NonPublic | BindingFlags.Instance); + method.Invoke(helper, null); + } + + [Test] + public void UpdateDisplay_NormalProtectedControl_IsVisibleWhenNotNormallyHidden() + { + SettingsProtectionSingleton.Settings.NormallyHidden = false; + using (var control = new Button { Visible = false }) + { + _helper.SetSettingsProtection(control, true); + + CallUpdateDisplay(_helper); + + Assert.That(control.Visible, Is.True); + } + } + + [Test] + public void UpdateDisplay_NormalProtectedControl_IsHiddenWhenNormallyHidden() + { + SettingsProtectionSingleton.Settings.NormallyHidden = true; + using (var control = new Button { Visible = true }) + { + _helper.SetSettingsProtection(control, true); + + CallUpdateDisplay(_helper); + + Assert.That(control.Visible, Is.False); + } + } + + [Test] + public void UpdateDisplay_AlwaysHiddenControl_RemainsHiddenWhenNotNormallyHidden() + { + // Even when NormallyHidden=false (i.e., normal controls are visible), + // an always-hidden control must stay hidden. + SettingsProtectionSingleton.Settings.NormallyHidden = false; + using (var control = new Button { Visible = true }) + { + _helper.SetSettingsProtection(control, true, keepHidden: true); + + CallUpdateDisplay(_helper); + + Assert.That(control.Visible, Is.False); + } + } + + [Test] + public void UpdateDisplay_AlwaysHiddenControl_RemainsHiddenWhenNormallyHidden() + { + SettingsProtectionSingleton.Settings.NormallyHidden = true; + using (var control = new Button { Visible = true }) + { + _helper.SetSettingsProtection(control, true, keepHidden: true); + + CallUpdateDisplay(_helper); + + Assert.That(control.Visible, Is.False); + } + } + + [Test] + public void SetSettingsProtection_AlwaysHiddenControl_IsHiddenWithoutWaitingForTimer() + { + SettingsProtectionSingleton.Settings.NormallyHidden = false; + using (var control = new Button { Visible = true }) + { + _helper.SetSettingsProtection(control, true, keepHidden: true); + + // Deliberately no UpdateDisplay call: an always-hidden control must not stay + // visible until the timer next fires. + Assert.That(control.Visible, Is.False); + } + } + + [Test] + public void SetSettingsProtection_SwitchFromAlwaysHiddenToNormal_ControlBecomesNormallyManaged() + { + SettingsProtectionSingleton.Settings.NormallyHidden = false; + using (var control = new Button { Visible = true }) + { + _helper.SetSettingsProtection(control, true, keepHidden: true); + CallUpdateDisplay(_helper); + Assert.That(control.Visible, Is.False, "Precondition: always-hidden should be hidden"); + + // Re-register without keepHidden — should now follow the normal rule + _helper.SetSettingsProtection(control, true, keepHidden: false); + CallUpdateDisplay(_helper); + + Assert.That(control.Visible, Is.True); + } + } + + [Test] + public void SetSettingsProtection_SwitchFromAlwaysHiddenToNormal_IsShownWithoutWaitingForTimer() + { + SettingsProtectionSingleton.Settings.NormallyHidden = false; + using (var control = new Button { Visible = true }) + { + _helper.SetSettingsProtection(control, true, keepHidden: true); + Assert.That(control.Visible, Is.False, "Precondition: always-hidden should be hidden"); + + _helper.SetSettingsProtection(control, true, keepHidden: false); + + // Deliberately no UpdateDisplay call: registering hid the control, so the normal + // rule must be applied without waiting for the timer. + Assert.That(control.Visible, Is.True); + } + } + + [Test] + public void SetSettingsProtection_SwitchFromAlwaysHiddenToNormal_StaysHiddenWhenNormallyHidden() + { + SettingsProtectionSingleton.Settings.NormallyHidden = true; + using (var control = new Button { Visible = true }) + { + _helper.SetSettingsProtection(control, true, keepHidden: true); + + _helper.SetSettingsProtection(control, true, keepHidden: false); + + Assert.That(control.Visible, Is.False); + } + } + + [Test] + public void SetSettingsProtection_UnprotectAlwaysHiddenControl_ControlBecomesVisible() + { + SettingsProtectionSingleton.Settings.NormallyHidden = false; + using (var control = new Button { Visible = true }) + { + _helper.SetSettingsProtection(control, true, keepHidden: true); + CallUpdateDisplay(_helper); + Assert.That(control.Visible, Is.False, "Precondition: always-hidden should be hidden"); + + _helper.SetSettingsProtection(control, false); + + Assert.That(control.Visible, Is.True); + } + } + + [Test] + public void GetSettingsProtection_AlwaysHiddenControl_ReturnsTrue() + { + using (var control = new Button()) + { + _helper.SetSettingsProtection(control, true, keepHidden: true); + + Assert.That(_helper.GetSettingsProtection(control), Is.True); + } + } + + [Test] + public void Dispose_WhileStillSitedInContainer_DoesNotThrow() + { + // Disposing removes the component from its container, which reads Site, and the Site + // override throws once disposed; so _isDisposed must not be set until that is done. + using (var container = new Container()) + { + var helper = new SettingsProtectionHelper(container); + + Assert.That(() => helper.Dispose(), Throws.Nothing); + } + } + } +} diff --git a/SIL.Windows.Forms/SettingProtection/SettingsProtectionHelper.Designer.cs b/SIL.Windows.Forms/SettingProtection/SettingsProtectionHelper.Designer.cs index 2de64280c..d32160100 100644 --- a/SIL.Windows.Forms/SettingProtection/SettingsProtectionHelper.Designer.cs +++ b/SIL.Windows.Forms/SettingProtection/SettingsProtectionHelper.Designer.cs @@ -15,17 +15,21 @@ partial class SettingsProtectionHelper /// true if managed resources should be disposed; otherwise, false. protected override void Dispose(bool disposing) { - if (disposing && (components != null)) + System.Diagnostics.Debug.WriteLineIf(!disposing, "****** Missing Dispose() call for " + GetType().Name + ". ****** "); + if (_isDisposed) + return; + if (disposing) { - components.Dispose(); - } - base.Dispose(disposing); - _checkForCtrlKeyTimer.Dispose(); - if(!_isDisposed) - { - _isDisposed = true; + if (components != null) + components.Dispose(); _componentsUnderSettingsProtection.Clear(); + _alwaysHiddenComponents.Clear(); } + // Must run before _isDisposed is set: the base implementation removes this component + // from its container, which reads the Site property, and our override of Site throws + // once disposed. + base.Dispose(disposing); + _isDisposed = true; } #region Component Designer generated code diff --git a/SIL.Windows.Forms/SettingProtection/SettingsProtectionHelper.cs b/SIL.Windows.Forms/SettingProtection/SettingsProtectionHelper.cs index 420f27c97..4dee64321 100644 --- a/SIL.Windows.Forms/SettingProtection/SettingsProtectionHelper.cs +++ b/SIL.Windows.Forms/SettingProtection/SettingsProtectionHelper.cs @@ -21,7 +21,9 @@ namespace SIL.Windows.Forms.SettingProtection [ProvideProperty("SettingsProtection", typeof(Control))] public partial class SettingsProtectionHelper : Component, IExtenderProvider { - private readonly HashSet _componentsUnderSettingsProtection; + private readonly HashSet _componentsUnderSettingsProtection = new HashSet(); + private readonly HashSet _alwaysHiddenComponents = new HashSet(); + private readonly bool _isRuntime; private bool _isDisposed; public bool CanExtend(object extendee) @@ -34,9 +36,8 @@ public SettingsProtectionHelper(IContainer container) { InitializeComponent(); - _componentsUnderSettingsProtection = new HashSet(); - - if (LicenseManager.UsageMode != LicenseUsageMode.Designtime) + _isRuntime = LicenseManager.UsageMode != LicenseUsageMode.Designtime; + if (_isRuntime) { container?.Add(this); _checkForCtrlKeyTimer.Enabled = true; @@ -63,19 +64,32 @@ public DialogResult LaunchSettingsIfAppropriate(Func settingsLaunc return result; } + private static bool ProtectedComponentsAreVisible + { + get + { + var keys = Keys.Control | Keys.Shift; + return !SettingsProtectionSingleton.Settings.NormallyHidden || + ((Control.ModifierKeys & keys) == keys); + } + } + private void UpdateDisplay() { if (_componentsUnderSettingsProtection == null)//sometimes get a tick before this has been set return; - var keys = Keys.Control | Keys.Shift; + var visible = ProtectedComponentsAreVisible; foreach (var component in _componentsUnderSettingsProtection) { - bool visible = !SettingsProtectionSingleton.Settings.NormallyHidden || ((Control.ModifierKeys & keys) == keys); - ShowOrHideComponent(component, visible); } + + foreach (var component in _alwaysHiddenComponents) + { + ShowOrHideComponent(component, false); + } } private static void ShowOrHideComponent(Component component, bool visible) @@ -89,7 +103,8 @@ private static void ShowOrHideComponent(Component component, bool visible) "Only components which are Controls or ToolStripItems can be under settings protection."); } - private void SetSettingsProtectionInternal(Component controlOrToolStripItem, bool isProtected) + private void SetSettingsProtectionInternal(Component controlOrToolStripItem, bool isProtected, + bool keepHidden = false) { if (controlOrToolStripItem == null) throw new ArgumentNullException(); @@ -100,13 +115,29 @@ private void SetSettingsProtectionInternal(Component controlOrToolStripItem, boo if (isProtected) { - _componentsUnderSettingsProtection.Add(controlOrToolStripItem); - // No need to call ShowOrHideComponent explicitly. It will get called when the - // timer fires. + if (keepHidden) + { + _alwaysHiddenComponents.Add(controlOrToolStripItem); + _componentsUnderSettingsProtection.Remove(controlOrToolStripItem); + // Hide now rather than on the next tick, except at design time, where the + // component must stay visible on the design surface. + if (_isRuntime) + ShowOrHideComponent(controlOrToolStripItem, false); + } + else + { + _componentsUnderSettingsProtection.Add(controlOrToolStripItem); + var wasAlwaysHidden = _alwaysHiddenComponents.Remove(controlOrToolStripItem); + // If it was always-hidden then it is hidden now, so apply the normal rule + // instead of leaving it hidden until the timer fires. + if (wasAlwaysHidden && _isRuntime) + ShowOrHideComponent(controlOrToolStripItem, ProtectedComponentsAreVisible); + } } else { _componentsUnderSettingsProtection.Remove(controlOrToolStripItem); + _alwaysHiddenComponents.Remove(controlOrToolStripItem); ShowOrHideComponent(controlOrToolStripItem, true); } } @@ -124,12 +155,27 @@ public bool GetSettingsProtection(Control c) if (c == null) throw new ArgumentNullException(); - return _componentsUnderSettingsProtection.Contains(c); + return _componentsUnderSettingsProtection.Contains(c) || _alwaysHiddenComponents.Contains(c); } [PublicAPI] public void SetSettingsProtection(Control c, bool isProtected) => SetSettingsProtectionInternal(c, isProtected); + + /// + /// Makes a control protected (i.e., managed) or not, optionally keeping it hidden even + /// when the other protected controls are revealed. + /// + /// The control to protect or stop protecting + /// Whether the control is under settings protection + /// When true and is + /// true, the control is hidden and stays hidden even while the user holds down + /// Ctrl+Shift to reveal the other protected controls. This has no effect when + /// is false. + /// c was null + [PublicAPI] + public void SetSettingsProtection(Control c, bool isProtected, bool keepHidden) => + SetSettingsProtectionInternal(c, isProtected, keepHidden); #endregion #region IComponent Members @@ -182,5 +228,20 @@ public void ManageComponent(Component controlOrToolStripItem) => [PublicAPI] public void SetSettingsProtection(ToolStripItem toolStripItem, bool isProtected) => SetSettingsProtectionInternal(toolStripItem, isProtected); + + /// + /// Allows you to dynamically make a ToolStripItem protected (i.e., managed) or not, + /// optionally keeping it hidden even when the other protected items are revealed + /// + /// The item to protect or stop protecting + /// Whether the item is under settings protection + /// When true and is + /// true, the item is hidden and stays hidden even while the user holds down + /// Ctrl+Shift to reveal the other protected items. This has no effect when + /// is false. + /// toolStripItem was null + [PublicAPI] + public void SetSettingsProtection(ToolStripItem toolStripItem, bool isProtected, bool keepHidden) => + SetSettingsProtectionInternal(toolStripItem, isProtected, keepHidden); } }