SettingsProtectionHelper: support always-hidden controls

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<T>(Func<T>)` 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
 

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);
+			}
+		}
+	}
+}

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<Component> _componentsUnderSettingsProtection;
+		private readonly HashSet<Component> _componentsUnderSettingsProtection = new HashSet<Component>();
+		private readonly HashSet<Component> _alwaysHiddenComponents = new HashSet<Component>();
+		private readonly bool _isRuntime;
 		private bool _isDisposed;
 
 		public bool CanExtend(object extendee)
@@ -34,9 +36,8 @@ public SettingsProtectionHelper(IContainer container)
 		{
 			InitializeComponent();
 
-			_componentsUnderSettingsProtection = new HashSet<Component>();
-
-			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<DialogResult> 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);
+
+		/// <summary>
+		/// Makes a control protected (i.e., managed) or not, optionally keeping it hidden even
+		/// when the other protected controls are revealed.
+		/// </summary>
+		/// <param name="c">The control to protect or stop protecting</param>
+		/// <param name="isProtected">Whether the control is under settings protection</param>
+		/// <param name="keepHidden">When <c>true</c> and <paramref name="isProtected"/> is
+		/// <c>true</c>, 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
+		/// <paramref name="isProtected"/> is <c>false</c>.</param>
+		/// <exception cref="ArgumentNullException">c was null</exception>
+		[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);
+
+		/// <summary>
+		/// 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
+		/// </summary>
+		/// <param name="toolStripItem">The item to protect or stop protecting</param>
+		/// <param name="isProtected">Whether the item is under settings protection</param>
+		/// <param name="keepHidden">When <c>true</c> and <paramref name="isProtected"/> is
+		/// <c>true</c>, 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
+		/// <paramref name="isProtected"/> is <c>false</c>.</param>
+		/// <exception cref="ArgumentNullException">toolStripItem was null</exception>
+		[PublicAPI]
+		public void SetSettingsProtection(ToolStripItem toolStripItem, bool isProtected, bool keepHidden) =>
+			SetSettingsProtectionInternal(toolStripItem, isProtected, keepHidden);
 	}
 }