+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

4 files ±0 4 suites ±0 10m 12s ⏱️ -1s
5 095 tests ±0 4 861 ✅ - 1 234 💤 +1 0 ❌ ±0
16 597 runs ±0 15 876 ✅ - 2 721 💤 +2 0 ❌ ±0

Results for commit 751a8f3. ± 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-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).

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).

# 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.

… 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

+      <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.

This concern requires two simultaneous conditions:

GLOBAL_ANALYTICS_SETTING is non-null (the shared component is installed for this product)

The Privacy dialog runs and the user unchecks the global checkbox → SIL_ANALYTICS_ENABLED_GLOBALLY becomes null → Transitive schedules removal

But condition 2 can't happen when condition 1 is true, because AnalyticsNavigation.wxs only routes to PrivacyDlg when NOT PRODUCT_ANALYTICS_SETTING AND NOT GLOBAL_ANALYTICS_SETTING. If the global key exists, the dialog is bypassed entirely, the normalization CAs set SIL_ANALYTICS_ENABLED_GLOBALLY from the existing value, and the component condition stays TRUE throughout. The shared component's reference count is never at risk from this code path.

…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

+<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.

I think this is desirable. In practice, it really shouldn't matter much since these are just test apps. In theory, it could affect another developer's experience in minor ways when re-running a test app, but I doubt it will. Going forward, it just makes more sense for each app to have a distinct identity.

Properly parameterized and documented some overridable installer values. Added targets to compile the WiX fragments with candle.exe to catch syntax and schema errors early. Also fixed race condition that causes net462 build to fail

tombogle · 2026-04-20T18:28:40Z

+      <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>


This concern requires two simultaneous conditions:

GLOBAL_ANALYTICS_SETTING is non-null (the shared component is installed for this product)

The Privacy dialog runs and the user unchecks the global checkbox → SIL_ANALYTICS_ENABLED_GLOBALLY becomes null → Transitive schedules removal

But condition 2 can't happen when condition 1 is true, because AnalyticsNavigation.wxs only routes to PrivacyDlg when NOT PRODUCT_ANALYTICS_SETTING AND NOT GLOBAL_ANALYTICS_SETTING. If the global key exists, the dialog is bypassed entirely, the normalization CAs set SIL_ANALYTICS_ENABLED_GLOBALLY from the existing value, and the component condition stays TRUE throughout. The shared component's reference count is never at risk from this code path.

tombogle

@tombogle made 2 comments and resolved 3 discussions.
Reviewable status: 0 of 31 files reviewed, 1 unresolved discussion (waiting on andrew-polk, mark-sil, and megahirt).

tombogle · 2026-04-20T18:31:58Z

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


I think this is desirable. In practice, it really shouldn't matter much since these are just test apps. In theory, it could affect another developer's experience in minor ways when re-running a test app, but I doubt it will. Going forward, it just makes more sense for each app to have a distinct identity.

+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 AI found potential problems Mar 2, 2026

View reviewed changes

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

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

tombogle commented Mar 9, 2026

View reviewed changes

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

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

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

Comment thread SIL.Windows.Forms/Privacy/AnalyticsProxy.cs Outdated

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

Comment thread SIL.Installer/Analytics.wxs Outdated

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

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

Fixed edge cases related to repair installations.

a0a1895

Properly parameterized and documented some overridable installer values. Added targets to compile the WiX fragments with candle.exe to catch syntax and schema errors early. Also fixed race condition that causes net462 build to fail

github-advanced-security AI found potential problems Apr 20, 2026

View reviewed changes

Comment thread .github/workflows/build.yml Fixed

Added required explicit setup@v3 for execute@v3 action

76f7041

github-advanced-security AI found potential problems Apr 20, 2026

View reviewed changes

Comment thread .github/workflows/build.yml Fixed

Pinned GitVersion actions to specific SHA

751a8f3

tombogle commented Apr 20, 2026

View reviewed changes

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!

Uh oh!

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!

Uh oh!

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!

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 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tombogle Apr 20, 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!

tombogle Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tombogle Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

tombogle left a comment

Choose a reason for hiding this comment

Uh oh!

tombogle Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

tombogle commented Mar 2, 2026 •

edited

Loading

github-actions Bot commented Mar 2, 2026 •

edited

Loading

devin-ai-integration Bot Mar 26, 2026 •

edited

Loading