+semver:minor Added PrivacyDlg to allow control over analytics tracking by tombogle · Pull Request #1493 · sillsdev/libpalaso

tombogle · 2026-03-02T06:13:24Z

Added new SIL.Installer package for common installer components. Includes a Privacy dialog to enable users to opt out of analytics reporting.
Added two public members currently just used in Test App but potentially useful elsewhere:

PathUtilities.ParentDirectories extension method.
FileLocationUtilities.DistFilesFolderPath property.

Added elements to TestApp to exercise the new Privacy dialog

Added installer for SIL.Windows.Forms.TestApp - not intended for deployment, just a testbed for the wxs files in SIL.Installer

Added elements to TestApp to exercise the new Privacy dialog

This change is

…tics tracking is allowed Added elements to TestApp to exercise the new Privacy dialog

devin-ai-integration

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 5 additional findings.

github-actions · 2026-03-02T06:21:12Z

Palaso Tests

3 files - 1 3 suites - 1 6m 42s ⏱️ - 3m 29s
5 095 tests ± 0 4 861 ✅ - 1 234 💤 + 1 0 ❌ ±0
11 483 runs - 5 114 11 009 ✅ - 4 869 474 💤 - 245 0 ❌ ±0

Results for commit 4067f22. ± Comparison against base commit ce83cef.

This pull request skips 1 test.

SIL.Tests.IO.FileLocationUtilitiesTests ‑ LocateInProgramFiles_SendValidProgramDeepSearch_ReturnsProgramPath

♻️ This comment has been updated with latest results.

tombogle · 2026-03-09T12:57:36Z

SIL.Windows.Forms/Privacy/PrivacyDlg.Designer.cs

+            // 
+            // _buttonOK
+            // 
+            this._buttonOK.Anchor = ((System.Windows.Forms.AnchorStyles)((System.Windows.Forms.AnchorStyles.Bottom | System.Windows.Forms.AnchorStyles.Right)));


To fix the problem in general, remove explicit casts where the compiler already infers the same type for the expression, as they add clutter without changing semantics. For enum flag combinations (like AnchorStyles), the result of a bitwise OR of enum values is already of that enum type, so an explicit cast to the same enum is redundant.

In this specific case in SIL.Windows.Forms/Privacy/PrivacyDlg.Designer.cs, line 138 assigns to _buttonOK.Anchor using:

this._buttonOK.Anchor = ((System.Windows.Forms.AnchorStyles)((System.Windows.Forms.AnchorStyles.Bottom | System.Windows.Forms.AnchorStyles.Right)));

Here, (System.Windows.Forms.AnchorStyles.Bottom | System.Windows.Forms.AnchorStyles.Right) is already of type System.Windows.Forms.AnchorStyles, so the outer cast ((System.Windows.Forms.AnchorStyles)( ... )) can be removed safely. The best fix is to replace the entire assignment with a direct use of the bitwise OR expression, preserving all other properties and layout code unchanged. No new methods, imports, or definitions are required; we only simplify this one assignment expression.

True, but this is generated Designer code.

tombogle · 2026-03-09T12:57:37Z

SIL.Windows.Forms/Privacy/PrivacyDlg.Designer.cs

+            // 
+            // _buttonCancel
+            // 
+            this._buttonCancel.Anchor = ((System.Windows.Forms.AnchorStyles)((System.Windows.Forms.AnchorStyles.Bottom | System.Windows.Forms.AnchorStyles.Right)));


In general, to fix a "cast to same type" issue, remove the explicit cast when the expression already has the target type and no overload resolution or nullability semantics depend on the cast. This reduces noise and potential confusion without changing behavior.

Here, both _buttonOK.Anchor (line 138) and _buttonCancel.Anchor (line 155) are assigned values wrapped in a redundant cast:

this._buttonCancel.Anchor = ((System.Windows.Forms.AnchorStyles)((System.Windows.Forms.AnchorStyles.Bottom | System.Windows.Forms.AnchorStyles.Right)));

The expression (System.Windows.Forms.AnchorStyles.Bottom | System.Windows.Forms.AnchorStyles.Right) already has type System.Windows.Forms.AnchorStyles. Assigning that directly to the Anchor property is sufficient. The best fix is to remove both outer and inner casts and assign the combined enum values directly, preserving the anchor behavior.

Concretely:

On line 155, change the right-hand side to System.Windows.Forms.AnchorStyles.Bottom | System.Windows.Forms.AnchorStyles.Right.

Make the same simplification on line 138 for _buttonOK.Anchor to keep the code consistent.

No new methods, imports, or definitions are needed; we are only simplifying the enum expression.

True, but this is generated Designer code.

tombogle · 2026-03-02T14:46:55Z

tombogle · 2026-03-02T14:47:15Z

tombogle

@tombogle made 2 comments and resolved 2 discussions.
Reviewable status: 0 of 11 files reviewed, all discussions resolved (waiting on andrew-polk, mark-sil, and megahirt).

tombogle · 2026-03-09T12:57:36Z

SIL.Windows.Forms/Privacy/PrivacyDlg.Designer.cs

+            // 
+            // _buttonOK
+            // 
+            this._buttonOK.Anchor = ((System.Windows.Forms.AnchorStyles)((System.Windows.Forms.AnchorStyles.Bottom | System.Windows.Forms.AnchorStyles.Right)));


True, but this is generated Designer code.

tombogle · 2026-03-09T12:57:37Z

SIL.Windows.Forms/Privacy/PrivacyDlg.Designer.cs

+            // 
+            // _buttonCancel
+            // 
+            this._buttonCancel.Anchor = ((System.Windows.Forms.AnchorStyles)((System.Windows.Forms.AnchorStyles.Bottom | System.Windows.Forms.AnchorStyles.Right)));


True, but this is generated Designer code.

Fixed incomplete comment.

tombogle

@tombogle made 1 comment.
Reviewable status: 0 of 12 files reviewed, 3 unresolved discussions (waiting on andrew-polk, mark-sil, and megahirt).

SIL.Windows.Forms/Privacy/AnalyticsProxy.cs

# Conflicts: # CHANGELOG.md

…omponents. Includes a Privacy dialog to enable users to opt out of analytics reporting. Added PathUtilities.ParentDirectories extension method. Added FileLocationUtilities.DistFilesFolderPath extension method. Added installer for SIL.Windows.Forms.TestApp - not intended for deployment, just a testbed for the wxs files in SIL.Installer

# Conflicts: # CHANGELOG.md

…rPath set by app. This addresses a concern raised by Devin (which in practice is probably not really a problem).

devin-ai-integration

Devin Review found 2 new potential issues.

View 16 additional findings in Devin Review.

SIL.Installer/Analytics.wxs

devin-ai-integration · 2026-03-26T21:35:51Z

TestApps/SIL.Windows.Forms.TestApp/SIL.Windows.Forms.TestApp.csproj

-    <Description>SIL.Windows.Forms.TestApp test app</Description>
-    <OutputPath>../../output/$(Configuration)</OutputPath>
+    <Description>SIL.Windows.Forms test app</Description>
+    <OutputPath>output/$(Configuration)</OutputPath>


🚩 TestApp output path changed — may affect CI or other build integrations

The OutputPath for SIL.Windows.Forms.TestApp was changed from ../../output/$(Configuration) (the shared repo output directory) to output/$(Configuration) (a project-local directory). This was likely done to support the new WiX installer test project which references .\..\SIL.Windows.Forms.TestApp\output\$(Configuration)\net48\. Any existing scripts or CI steps that depend on finding the TestApp binaries in the shared output/ directory at the repo root would need updating.

Was this helpful? React with 👍 or 👎 to provide feedback.

… reinstall Fixed problem of shared fixed GUID being used across all products

devin-ai-integration

Devin Review found 1 new potential issue.

View 19 additional findings in Devin Review.

devin-ai-integration · 2026-03-26T22:11:01Z

SIL.Installer/Analytics.wxs

+      <Component Id="GlobalAnalyticsSetting" Guid="{5D729D52-F0E1-443F-AA40-664AA754F1C1}">
+        <Condition><![CDATA[SIL_ANALYTICS_APPLY_GLOBALLY = "1"]]></Condition>
+        <RegistryKey Root="HKCU" Key="$(var.GlobalAnalyticsRegistryKey)">
+          <RegistryValue
+            Name="Enabled"
+            Type="integer"
+            Value="[SIL_ANALYTICS_ENABLED_GLOBALLY]"
+            KeyPath="yes" />
+        </RegistryKey>
+      </Component>


🚩 WiX GlobalAnalyticsSetting component uses fixed GUID shared across all SIL products

The GlobalAnalyticsSetting component at line 81 uses a hardcoded GUID {5D729D52-F0E1-443F-AA40-664AA754F1C1} so all SIL products share ownership of the global Software\SIL\Analytics registry key. MSI reference counting means the key is only removed when the last product referencing it is uninstalled. However, the component condition SIL_ANALYTICS_APPLY_GLOBALLY = "1" is evaluated per-product on each install/upgrade. If a product was previously installed with this condition true (global applied) and on reinstall the condition becomes false (e.g., user set a product-specific override via the C# dialog), MSI could deregister this product's reference to the shared component. If no other SIL product is installed, this would delete the global registry key. This edge case is mitigated by the fact that the C# AnalyticsProxy.Update method (not the installer) is the primary mechanism for changing settings at runtime.

Was this helpful? React with 👍 or 👎 to provide feedback.

…not set for the current user in the registry

devin-ai-integration

Devin Review found 1 new potential issue.

View 19 additional findings in Devin Review.

devin-ai-integration · 2026-03-27T22:18:52Z

TestApps/Directory.Build.props

+<Project>
+  <Import Project="..\Directory.Build.props" />
+
+  <PropertyGroup>
+    <Product>$(AssemblyName)</Product>
+  </PropertyGroup>
+</Project>


🚩 New TestApps/Directory.Build.props changes Product metadata for all test apps

This new file overrides <Product> from libpalaso (root Directory.Build.props:7) to $(AssemblyName) for all projects under TestApps/. This affects Application.ProductName in WinForms apps. Currently only SIL.Windows.Forms.TestApp uses Application.ProductName (at Program.cs:72 for the AnalyticsProxy). Other test apps under TestApps/ (ArchivingTestApp, ClipboardTestApp, etc.) don't reference Application.ProductName, so they're not affected functionally. However, their assembly metadata will change, which could affect things like user settings file paths (Application.UserAppDataPath includes ProductName).

Was this helpful? React with 👍 or 👎 to provide feedback.

+semver:minor Added PrivacyDlg to allow user to control whether analy…

a5fdc13

…tics tracking is allowed Added elements to TestApp to exercise the new Privacy dialog

tombogle requested review from andrew-polk, mark-sil and megahirt March 2, 2026 06:13

tombogle self-assigned this Mar 2, 2026

tombogle added the enhancement label Mar 2, 2026

devin-ai-integration bot reviewed Mar 2, 2026

View reviewed changes

github-advanced-security bot found potential problems Mar 2, 2026

View reviewed changes

tombogle commented Mar 9, 2026

View reviewed changes

This comment was marked as resolved.

Sign in to view

Fixed a couple minor issues raised by Devin review

fe4d560

This comment was marked as resolved.

Sign in to view

Improved logic when clicking OK, especially in case where saving fails.

20d8a98

Fixed incomplete comment.

tombogle commented Mar 19, 2026

View reviewed changes

SIL.Windows.Forms/Privacy/AnalyticsProxy.cs Outdated Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

tombogle added 3 commits March 19, 2026 18:11

Merge branch 'master' into add-dlg-for-analytics-opt-out

52da232

# Conflicts: # CHANGELOG.md

Fixed LocalizationManagerId in PrivacyDlg

d4c0713

tombogle force-pushed the add-dlg-for-analytics-opt-out branch from b81717a to a579667 Compare March 26, 2026 05:12

Merge branch 'master' into add-dlg-for-analytics-opt-out

0b56e45

# Conflicts: # CHANGELOG.md

This comment was marked as resolved.

Sign in to view

tombogle added 2 commits March 26, 2026 08:56

Fixed WIX toolset install

84d44ea

Prevented overwrite (for internal caching purposes) of DistFilesFolde…

2a0fbfa

…rPath set by app. This addresses a concern raised by Devin (which in practice is probably not really a problem).

tombogle force-pushed the add-dlg-for-analytics-opt-out branch from 4cc5955 to 38929ff Compare March 26, 2026 21:14

Correct fix for ensuring compatible WIX toolset

df546f8

tombogle force-pushed the add-dlg-for-analytics-opt-out branch from 38929ff to df546f8 Compare March 26, 2026 21:24

devin-ai-integration bot reviewed Mar 26, 2026

View reviewed changes

Fixed silent escalation of product-level opt-out to global opt-out on…

64ba5b6

… reinstall Fixed problem of shared fixed GUID being used across all products

devin-ai-integration bot reviewed Mar 26, 2026

View reviewed changes

tombogle added 2 commits March 27, 2026 09:33

Merge branch 'master' into add-dlg-for-analytics-opt-out

cb2a490

Removed code to attempt to ensure that WiX 3.14 is installed

b547a23

This comment was marked as resolved.

Sign in to view

Implemented navigation in repair scenario when Analytics options are …

4067f22

…not set for the current user in the registry

devin-ai-integration bot reviewed Mar 27, 2026

View reviewed changes

@@ -135,7 +135,7 @@
                         //
                         // _buttonOK
                         //
-                        this._buttonOK.Anchor = ((System.Windows.Forms.AnchorStyles)((System.Windows.Forms.AnchorStyles.Bottom | System.Windows.Forms.AnchorStyles.Right)));
+                        this._buttonOK.Anchor = System.Windows.Forms.AnchorStyles.Bottom | System.Windows.Forms.AnchorStyles.Right;
                         this._buttonOK.AutoSize = true;
                         this.locExtender.SetLocalizableToolTip(this._buttonOK, null);
                         this.locExtender.SetLocalizationComment(this._buttonOK, null);

@@ -135,7 +135,7 @@
                         //
                         // _buttonOK
                         //
-                        this._buttonOK.Anchor = ((System.Windows.Forms.AnchorStyles)((System.Windows.Forms.AnchorStyles.Bottom | System.Windows.Forms.AnchorStyles.Right)));
+                        this._buttonOK.Anchor = System.Windows.Forms.AnchorStyles.Bottom | System.Windows.Forms.AnchorStyles.Right;
                         this._buttonOK.AutoSize = true;
                         this.locExtender.SetLocalizableToolTip(this._buttonOK, null);
                         this.locExtender.SetLocalizationComment(this._buttonOK, null);
@@ -152,7 +152,7 @@
                         //
                         // _buttonCancel
                         //
-                        this._buttonCancel.Anchor = ((System.Windows.Forms.AnchorStyles)((System.Windows.Forms.AnchorStyles.Bottom | System.Windows.Forms.AnchorStyles.Right)));
+                        this._buttonCancel.Anchor = System.Windows.Forms.AnchorStyles.Bottom | System.Windows.Forms.AnchorStyles.Right;
                         this._buttonCancel.AutoSize = true;
                         this._buttonCancel.DialogResult = System.Windows.Forms.DialogResult.Cancel;
                         this.locExtender.SetLocalizableToolTip(this._buttonCancel, null);

Uh oh!

Conversation

tombogle commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devin-ai-integration bot left a comment

Choose a reason for hiding this comment

✅ Devin Review: No Issues Found

Uh oh!

github-actions bot commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Palaso Tests

Uh oh!

Check warning

Copilot Autofix

tombogle Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Check warning

Copilot Autofix

tombogle Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

tombogle commented Mar 2, 2026

Uh oh!

tombogle commented Mar 2, 2026

Uh oh!

tombogle left a comment

Choose a reason for hiding this comment

Uh oh!

tombogle Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

tombogle Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

tombogle left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

devin-ai-integration bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

devin-ai-integration bot Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot left a comment

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

devin-ai-integration bot left a comment

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

tombogle commented Mar 2, 2026 •

edited

Loading

github-actions bot commented Mar 2, 2026 •

edited

Loading