Skip to content

Add resource-DLL localization foundation - #2994

Open
creamtea47 wants to merge 3 commits into
winsiderss:masterfrom
creamtea47:feature/localization-foundation
Open

Add resource-DLL localization foundation#2994
creamtea47 wants to merge 3 commits into
winsiderss:masterfrom
creamtea47:feature/localization-foundation

Conversation

@creamtea47

@creamtea47 creamtea47 commented Jul 14, 2026

Copy link
Copy Markdown

Summary

This PR implements the smallest reviewable localization foundation discussed in #302 and in the design comment.

  • keep the built-in English strings as the default and per-string fallback;
  • load an optional translations/SystemInformer.<locale>.dll with LOAD_LIBRARY_AS_DATAFILE | LOAD_LIBRARY_AS_IMAGE_RESOURCE, so no translation DLL code is executed;
  • add a Language setting (auto, an English locale, or an explicit locale name; restart required);
  • use Authenticode same-signer verification for release builds when KSI support is enabled; no KPH/plugin-signing verification is used for the data-only resource DLL;
  • allow unsigned resources in Debug and KSI-disabled community builds for local translation development;
  • add a minimal resource-only zh-CN project containing only the five top-level main-menu labels;
  • export the lookup API so first-party plugin integration can be considered in a follow-up.

Missing DLLs, invalid locale names, missing resource IDs, and English locale selections all take the fast English fallback path.

Scope

This intentionally does not attempt to translate every dialog or plugin in one change. First-party plugin resources, a user-facing language picker, packaging/signing integration, and broader string migration are deferred until the resource format and trust policy are accepted.

Existing plugin loading and menu-location identifiers are unchanged. As a compatibility check before the latest upstream environment-dialog migration, the first-party ToolStatus plugin built against the generated Debug x64 SDK.

Validation

  • Rebased without conflicts onto upstream master at 37501a3 (2026-07-20).
  • Debug x64 and Release x64 SystemInformer.zh-CN.dll builds: passed after the rebase.
  • Release x64 SystemInformer compile, including localization.c: passed after the rebase with a command-line-only SpectreMitigation=false override.
  • The translation DLL has an x64 machine type and a zero PE entry point.
  • Runtime auto selection on a zh-CN Windows profile: native menu text returned 系统 / 界面 / 工具 / 用户 / 帮助.
  • Runtime with the translation DLL absent: native menu text returned System / View / Tools / Users / Help without a crash.
  • git diff --check: passed.

A current full Visual Studio link is blocked by the upstream envdlg move: SystemInformer.vcxproj does not yet include SystemInformer/envdlg.c, and plugins/ExtendedTools/ExtendedTools.vcxproj still references the removed plugin copy. This produces unresolved PhShowEnvironmentVariablesDialog/PhShowEnvironmentVariableSplitDialog symbols in the main project and a missing envdlg.c in ExtendedTools; neither project-file mismatch is introduced by this PR. The local Visual Studio installation also lacks the x64 Spectre-mitigated libraries, so Release validation uses the command-line-only override above; no project security setting was changed.

Review questions

  1. Authenticode rather than KPH signing is now the documented direction for the data-only translation resource, following maintainer feedback. The current implementation enforces same-signer verification when KSI support is enabled in Release; should it instead be required for every Release build?
  2. Is a fixed shared string-ID range acceptable for the core application and future first-party plugin resources?
  3. Should the user-facing language picker and official signing/packaging changes be part of this PR, or follow after the loader design is accepted?

Related to #302.

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@creamtea47

Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

@creamtea47
creamtea47 marked this pull request as ready for review July 14, 2026 04:06
@creamtea47
creamtea47 requested review from dmex and jxy-s as code owners July 14, 2026 04:06
@dmex
dmex requested a review from Copilot July 14, 2026 21:28
@dmex dmex self-assigned this Jul 14, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a foundational localization mechanism for System Informer by loading optional, resource-only translation DLLs (no code execution) and exposing a lookup API that supports fallback to built-in English strings.

Changes:

  • Add a localization loader (PhInitializeLocalization) and string lookup API (PhGetLocalizedString) with a Language setting (auto or explicit locale).
  • Update the main menu to use localized string IDs with per-string English fallbacks.
  • Add an initial resource-only zh-CN translation DLL project and solution integration.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
translations/zh-CN/SystemInformer.zh-CN.vcxproj Adds a resource-only translation DLL project (NoEntryPoint) that outputs to the app’s translations\ folder.
translations/zh-CN/SystemInformer.zh-CN.rc Defines initial zh-CN stringtable entries for the top-level main menu labels.
translations/README.md Documents translation DLL behavior and fallback strategy.
SystemInformer/SystemInformer.vcxproj.filters Adds the new localization source/header files to the VS filters.
SystemInformer/SystemInformer.vcxproj Adds localization.c and localization headers to the main project build.
SystemInformer/SystemInformer.def Exports the localization initialization and string lookup APIs for consumers (e.g., plugins).
SystemInformer/settings.c Adds default Language=auto setting.
SystemInformer/mainwnd.c Switches top-level main menu labels to use PhGetLocalizedString with English fallbacks.
SystemInformer/main.c Initializes localization during startup.
SystemInformer/localization.c Implements translation DLL selection, trust policy, resource loading, and caching.
SystemInformer/include/phsettings.h Adds SETTING_LANGUAGE constant.
SystemInformer/include/phapp.h Includes localization.h in the public app header.
SystemInformer/include/localizationres.h Introduces the shared localized string ID range and initial IDs.
SystemInformer/include/localization.h Declares the exported localization APIs.
SystemInformer.slnx Adds the SystemInformer.zh-CN translation project to the solution definition.
SystemInformer.sln Adds the SystemInformer.zh-CN translation project and build configurations to the VS solution.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread SystemInformer/localization.c
Comment thread SystemInformer/localization.c Outdated
Comment thread SystemInformer/localization.c
@creamtea47
creamtea47 force-pushed the feature/localization-foundation branch from a4a28d6 to f289092 Compare July 15, 2026 01:18
@creamtea47

Copy link
Copy Markdown
Author

Updated the branch onto current master and addressed all three review comments in f289092: RT_STRING bounds validation, PH_INITONCE initialization, and an English-first bilingual trust-policy comment. Local Debug x64 SystemInformer, Release x64 compilation, Debug/Release zh-CN resource DLLs, and Debug x64 ExtendedTools all pass. GitHub currently marks the two new CI workflow runs as action_required with no jobs, so they appear to require maintainer approval for the updated fork commit.

@jxy-s

jxy-s commented Jul 19, 2026

Copy link
Copy Markdown
Member

I have not fully reviewed this code but I want to stop by and give a response to the following question:

  1. Is the same-signer policy appropriate for official builds, or should this use the existing KPH plugin-signing mechanism instead of Authenticode verification?

We could leverage this but the verification code using the KPH signing does not exist in user-mode. And it should not be required for loading the localization DLL - as long as the localization DLL is loaded as a data-file (e.g. no-execute). We shouldn't apply or expect any KPH signing in this situation. An Authenticode check in release builds seems completely reasonable.

Introduce data-only translation resources with English fallback, locale selection through the Language setting, release trust checks, and a minimal zh-CN main-menu sample.
Validate RT_STRING block bounds, make localization initialization idempotent, and document the unsigned development path bilingually.
@creamtea47
creamtea47 force-pushed the feature/localization-foundation branch from f289092 to 7b0a5dc Compare July 20, 2026 02:37
@creamtea47

Copy link
Copy Markdown
Author

@jxy-s Thanks, that confirms the direction. The loader keeps the translation DLL data-only (LOAD_LIBRARY_AS_DATAFILE | LOAD_LIBRARY_AS_IMAGE_RESOURCE) and does not use KPH/plugin-signing verification.

The current implementation uses Authenticode same-signer verification in Release when KSI support is enabled, while Debug and KSI-disabled community builds can load unsigned resources for translation development. If you would prefer Authenticode enforcement for every Release build regardless of KSI, I can tighten that condition.

I have also rebased the PR onto the latest upstream master (37501a3).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants