Fix: Correct service name translations in dashboard widget

[tobiasdegen]

Hi,

I've fixed the translations of the service names shown in the "Services" widget on the dashboard.

Originally, the translations were defined in src/etc/inc/core.inc, but they were not applied because the language system is not initialized at that stage — there's no user context available yet.

Since the widget retrieves the translated strings directly via the API, the translations are now properly applied in that context.

However, I’ve left the gettext() calls in core.inc so that the strings are still included in the .po files and can be translated through the usual localization process.

Best regards,
Tobias

[AdSchellevis]

If there's an issue with the context, we should fix that at the caller, this will be quite confusing and prone to future mistakes. (assuming we don't want to remove the gettext() in the service definitions)

*** EDIT ***

The proper fix will likely be a bit more complicated to apply, ideally configd knows the proper language to apply, but as this may differ per user, it does need to track or pass the user. Food for thought I'm afraid.

[fichtner]

If we agree pluginctl doesn’t care about user language we can actually say this is the correct fix, although in that case we should try to wrap all the variables that are translated (not sure if there are more without looking at the code).

Cheers,
Franco

[tobiasdegen]

To understand this:
The gettext() call in the service definition is incorrect because the context is obviously missing (or irrelevant). The translation should happen at GUI runtime when a user is logged in.

In this pull request, the services are translated when they are called from the API (context is present).

An alternative would be to translate them directly in the widget. The translations for this are defined in the file src/opnsense/www/js/widgets/Metadata/Core.xml.

For example:

<translations>
    <System Configuration Daemon>System Configuration Daemon</System Configuration Daemon>
    <Cron>Cron</Cron>
    <ddclient>ddsclient</ddclient>
</translations>

But it's weird with the spaces and I don't know if it works.

You could also change the descriptions of the services in the core.inc to better map the translations.

</translations>
    <service.sysconfig>System Configuration Daemon</service.sysconfig>
    <service.cron>Cron</service.cron>
    <service.ddclient>ddclient</service.ddclient>
    <service.syslog>Syslog-ng Daemon</service.syslog>
    <service.ssh>Secure Shell Daemon</service.ssh>
    <service.unbound>Unbound DNS</service.unbound>
</translations>

With this approach, the translations would be defined, the .po files would be generated with them and gettext() could be removed in core.inc.

What do you think?

P.S. This is my first deeper dive into the OPNsense internals, so please feel free to point out anything I might have misunderstood or could improve.

[fichtner]

I double checked and the description is the only thing that is translated for services. Since all files already wrap their description in gettext() I believe this change is correct to move the pluginctl feed from the default language to the user's logged in language.

[AdSchellevis]

I agree the most logical place to handle the translations would indeed be the controller layer as we have all relevant context available there, in which case this would be correct but leaves the issue of the gettext() calls in the plugin code itself.

I'm struggling a bit with the pros and cons of moving the text to translate into separate files here. To some degree it feels simpler/cleaner to just stuff the translations in the file and not use the outcome. Which would lead to something like this:

   gettext('System Configuration Daemon'); /* feed translations */
   $services[] = array(
        'description' =>'System Configuration Daemon',
        'pidfile' => '/var/run/configd.pid',
        'mwexec' => array(
            'restart' => array('/usr/local/etc/rc.d/configd restart'),
            'start' => array('/usr/local/etc/rc.d/configd start'),
            'stop' => array('/usr/local/etc/rc.d/configd stop'),
        ),
        'name' => 'configd',
        'locked' => true,
    );

My main concern is trowing different than expected output when calling gettext() twice, someone might set environment variables which lead to a different outcome (and very vague tickets).

[tobiasdegen]

OK, how do we want to do it? Like in your example? Then I would do that.

[AdSchellevis]

@tobiasdegen we're a bit busy with the upcoming release and need time to discuss this internally. In the meantime we'll keep the PR open.

[tobiasdegen]

OK, all right.

[fichtner]

Discussed options a bit today:

https://www.gnu.org/software/gettext/manual/html_node/xgettext-Invocation.html

C and Perl support gettext_noop() and we can leverage this in our code to avoid backend side translations while still getting strings extracted. It requires a modification of the xgettext binary, but I think this a one line change.

[fichtner]

I did a short partial test with 039382f but it looks like work that's better not spent since it requires more targeted changes, ideally an upstream PHP and gettext support or at least more sed glue in lang.git in order to extract those things.

And then we still need to micro-manage every translatable string that should be treated as a "delayed" translation.