[PATCH cluster/manager v2 0/2] Configurable window titles for nodes

[PATCH cluster/manager v2 0/2] Configurable window titles for nodes

[Thomas Ellmenreich]

Thank you @Dominik Csapak and @Thomas Lamprecht for your comments.

Configurable window titles for nodes
====================================

Currently the window title is fixed to the following string:

$nodename - Proxmox Virtual Environment

This has become problematic for users with multiple clusters, each containing
the same node name. To help distinguish between tabs, it has been requested [1]
that the window title be made configurable with the fqdn as an option.

[1] https://bugzilla.proxmox.com/show_bug.cgi?id=5475

Implementation
--------------

After considering the different options, I have deemed a server side
implementation, through the datacenter.cfg, to be the better option.
Specifically because of its durability across sessions and because it
automatically configures the title for all users and all nodes across the
cluster.

The config now contains a new ui-settings format string which then in turn
contains a 'title' enum field for the window title. This enum string will then
be converted to a window title and inserted in the index.html.tpl template on
every request.

The current enum options are:

- default - Displays the default '$nodename - Proxmox Virtual Environment' string
  if the config enum is not present

- 'node-and-cluster' - Displays the nodename and then the clustername in the
  following style: '$nodename - $clustername - Proxmox Virtual Environment'

- 'fqdn' - Displays the current fqdn as returned by `PVE::Tools::get_fqdn()`
  like so: '$fqdn - Proxmox Virtual Environment'


Possible future extensions
--------------------------

- If proper cross-node user settings are implemented in the future, they could
  be used to allow users to define their own window title settings and have
  these settings be persisted across sessions

- One of the selection options for window titles could be a user defined
  template string which would allow users of PVE to define their own
  window titles.

Changes since v1
----------------

- Instead of storing the 'window-title' setting directly in the datacenter.cfg
  its now nested in a more general 'ui-settings' format string. This allows
  easier expansion in the future

- All adjustments that were needed to create the 'ui-settings' format string

Changes since RFC
-----------------

- change of the `datacenter.cfg` setting title from "windowtitle" to
  "window-title"

- renamed `nodeandcluster` to `node-and-cluster`

- added the " - Proxmox Virtual Environment" trailer to all titles

- removed the questions from this text and added the "Possible future
  extensions" section


pve-manager:

Thomas Ellmenreich (1):
  fix #5475: configurable window title

 PVE/Service/pveproxy.pm       | 21 ++++++++++++
 www/index.html.tpl            |  2 +-
 www/manager6/UIOptions.js     |  6 ++++
 www/manager6/dc/OptionView.js | 61 +++++++++++++++++++++++++++++++++++
 4 files changed, 89 insertions(+), 1 deletion(-)


pve-cluster:

Thomas Ellmenreich (1):
  fix #5475: configurable window title

 src/PVE/DataCenterConfig.pm | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)


Summary over all repositories:
  5 files changed, 114 insertions(+), 1 deletions(-)

-- 
Generated by murpp 0.12.0

[PATCH manager v2 1/2] fix #5475: configurable window title

[Thomas Ellmenreich]

Allow setting of the new 'ui-settings' format string and the nested
'title' property. Setting this property defines the window titles
for all cluster nodes.

Signed-off-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
---
 PVE/Service/pveproxy.pm       | 21 ++++++++++++
 www/index.html.tpl            |  2 +-
 www/manager6/UIOptions.js     |  6 ++++
 www/manager6/dc/OptionView.js | 61 +++++++++++++++++++++++++++++++++++
 4 files changed, 89 insertions(+), 1 deletion(-)

diff --git a/PVE/Service/pveproxy.pm b/PVE/Service/pveproxy.pm
index c6011a00..811295f4 100755
--- a/PVE/Service/pveproxy.pm
+++ b/PVE/Service/pveproxy.pm
@@ -202,6 +202,24 @@ my sub get_path_mtime {
     return $mtime;
 }
 
+# builds the window title from the datacenter.cfg configuration
+my $get_window_title = sub {
+    my ($title_enum, $nodename) = @_;
+
+    my $title = "$nodename";
+
+    if ($title_enum eq "node-and-cluster") {
+        my $clinfo = PVE::Cluster::get_clinfo();
+        my $clustername = $clinfo->{cluster}->{name};
+
+        $title = "$nodename - $clustername";
+    } elsif ($title_enum eq "fqdn") {
+        $title = PVE::Tools::get_fqdn();
+    }
+
+    return $title . " - Proxmox Virtual Environment";
+};
+
 # NOTE: Requests to those pages are not authenticated so we must be very careful here
 sub get_index {
     my ($nodename, $server, $r, $args) = @_;
@@ -232,10 +250,12 @@ sub get_index {
         }
     }
 
+    my $window_title;
     my $consent_text;
     eval {
         my $dc_conf = PVE::Cluster::cfs_read_file('datacenter.cfg');
         $consent_text = $dc_conf->{'consent-text'};
+        $window_title = $get_window_title->($dc_conf->{'ui-settings'}->{'title'}, $nodename);
 
         if (!$lang) {
             $lang = $dc_conf->{language} // 'en';
@@ -276,6 +296,7 @@ sub get_index {
         token => $token,
         console => $args->{console},
         nodename => $nodename,
+        window_title => $window_title,
         debug => $debug,
         version => "$version",
         wtversion => $wtversion,
diff --git a/www/index.html.tpl b/www/index.html.tpl
index 74ee02d9..5138b7cc 100644
--- a/www/index.html.tpl
+++ b/www/index.html.tpl
@@ -4,7 +4,7 @@
     <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
     <meta http-equiv="X-UA-Compatible" content="IE=edge">
     <meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1, user-scalable=no">
-    <title>[% nodename %] - Proxmox Virtual Environment</title>
+    <title>[% window_title %]</title>
     <link rel="icon" sizes="128x128" href="/pve2/images/logo-128.png" />
     <link rel="apple-touch-icon" sizes="128x128" href="/pve2/images/logo-128.png" />
     <link rel="stylesheet" type="text/css" href="/pve2/ext6/theme-crisp/resources/theme-crisp-all.css?ver=7.0.0" />
diff --git a/www/manager6/UIOptions.js b/www/manager6/UIOptions.js
index 8c4674af..bb0af8d5 100644
--- a/www/manager6/UIOptions.js
+++ b/www/manager6/UIOptions.js
@@ -90,6 +90,12 @@ Ext.define('PVE.UIOptions', {
         alphabetical: gettext('Alphabetical'),
     },
 
+    titleOptions: {
+        __default__: '$nodename - Proxmox Virtual Environment',
+        'node-and-cluster': '$nodename - $clustername - Proxmox Virtual Environment',
+        fqdn: '$fqdn - Proxmox Virtual Environment',
+    },
+
     shouldSortTags: function () {
         return !(PVE.UIOptions.options['tag-style']?.ordering === 'config');
     },
diff --git a/www/manager6/dc/OptionView.js b/www/manager6/dc/OptionView.js
index dc12aa7e..208cc147 100644
--- a/www/manager6/dc/OptionView.js
+++ b/www/manager6/dc/OptionView.js
@@ -91,6 +91,67 @@ Ext.define('PVE.dc.OptionView', {
             defaultValue: '__default__',
             deleteEmpty: true,
         });
+        me.rows['ui-settings'] = {
+            required: true,
+            renderer: (value) => {
+                if (value === undefined) {
+                    return gettext('No Overrides');
+                }
+                let txt = '';
+                if (value.title) {
+                    txt += Ext.String.format(gettext('Title: {0}'), value.title);
+                }
+                return txt;
+            },
+            header: gettext('Ui Settings'),
+            editor: {
+                xtype: 'proxmoxWindowEdit',
+                width: 800,
+                subject: gettext('Ui Settings'),
+                fieldDefaults: {
+                    labelWidth: 100,
+                },
+                url: '/api2/extjs/cluster/options',
+                items: [
+                    {
+                        xtype: 'inputpanel',
+                        setValues: function (values) {
+                            if (values === undefined) {
+                                return undefined;
+                            }
+                            values = values?.['ui-settings'] ?? {};
+                            values.title = values.title || '__default__';
+                            return Proxmox.panel.InputPanel.prototype.setValues.call(this, values);
+                        },
+                        onGetValues: function (values) {
+                            let style = {};
+                            if (values.title) {
+                                style.title = values.title;
+                            }
+                            let value = PVE.Parser.printPropertyString(style);
+                            if (value === '') {
+                                return {
+                                    delete: 'ui-settings',
+                                };
+                            }
+                            return {
+                                'ui-settings': value,
+                            };
+                        },
+                        items: [
+                            {
+                                xtype: 'proxmoxKVComboBox',
+                                name: 'title',
+                                fieldLabel: gettext('Title'),
+                                comboItems: Object.entries(PVE.UIOptions.titleOptions),
+                                deleteEmpty: true,
+                                defaultValue: '__default__',
+                            },
+                        ],
+                    },
+                ],
+            },
+        };
         me.add_text_row('email_from', gettext('Email from address'), {
             deleteEmpty: true,
             vtype: 'proxmoxMail',
-- 
2.47.3

[PATCH cluster v2 2/2] fix #5475: configurable window title

[Thomas Ellmenreich]

Added a new 'ui-settings' format string to the datacenter.cfg. Currently
only contains a 'title' option to determine the window title of the
nodes browser tab.

Signed-off-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
---
 src/PVE/DataCenterConfig.pm | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/src/PVE/DataCenterConfig.pm b/src/PVE/DataCenterConfig.pm
index 004122e..751d8ed 100644
--- a/src/PVE/DataCenterConfig.pm
+++ b/src/PVE/DataCenterConfig.pm
@@ -344,6 +344,16 @@ my $user_tag_privs_format = {
     },
 };
 
+my $ui_settings_format = {
+    'title' => {
+        optional => 1,
+        type => 'string',
+        enum => ['node-and-cluster', 'fqdn'],
+        description =>
+            "Define the text displayed in the browser tab title for all nodes in this cluster.",
+    },
+};
+
 my $datacenter_schema = {
     type => "object",
     additionalProperties => 0,
@@ -436,6 +446,12 @@ my $datacenter_schema = {
             # FIXME: remove 'applet' with 9.0 (add pve8to9 check!)
             enum => ['applet', 'vv', 'html5', 'xtermjs'],
         },
+        'ui-settings' => {
+            optional => 1,
+            type => "string",
+            format => $ui_settings_format,
+            description => "For cluster wide ui settings.",
+        },
         email_from => {
             optional => 1,
             type => 'string',
@@ -618,6 +634,10 @@ sub parse_datacenter_config {
         $res->{'tag-style'} = parse_property_string($tag_style_format, $tag_style);
     }
 
+    if (my $ui_settings = $res->{'ui-settings'}) {
+        $res->{'ui-settings'} = parse_property_string($ui_settings_format, $ui_settings);
+    }
+
     if (my $user_tag_privs = $res->{'user-tag-access'}) {
         $res->{'user-tag-access'} = parse_property_string($user_tag_privs_format, $user_tag_privs);
 
@@ -711,6 +731,11 @@ sub write_datacenter_config {
         $cfg->{'tag-style'} = PVE::JSONSchema::print_property_string($tag_style, $tag_style_format);
     }
 
+    if (ref(my $ui_settings = $cfg->{'ui-settings'})) {
+        $cfg->{'ui-settings'} =
+            PVE::JSONSchema::print_property_string($ui_settings, $ui_settings_format);
+    }
+
     if (ref(my $user_tag_privs = $cfg->{'user-tag-access'})) {
         if (my $user_tags = $user_tag_privs->{'user-allow-list'}) {
             $user_tag_privs->{'user-allow-list'} = join(';', sort $user_tags->@*);
-- 
2.47.3

Re: [PATCH manager v2 1/2] fix #5475: configurable window title

[Elias Huhsovitz]

On Mon Jul 20, 2026 at 12:03 PM CEST, Thomas Ellmenreich wrote:
> Allow setting of the new 'ui-settings' format string and the nested
> 'title' property. Setting this property defines the window titles
> for all cluster nodes.
>
> Signed-off-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
TL;DR: 
- missing argument for PVE::Tools::get_fqdn($nodename)
- missing defined checks pveproxy.pm
- confusing option names

I tested the patch on a pve cluster consisting of 2 nodes.
1. The setting names render including the $ symbol, e.g. ($nodename, $fqdn) in the WebUI. 
This makes it seem broken from an end user point of view. 
You generally do not expect placeholder values in settings menu.
2. I encountered a bug regarding the fdqn setting (see comment below)
3. I recommend adding some general robustness in case of undef values:
Initialize $window_title to a safe value not relying on the cluster, in
case of an error. Expect that the underlying API/calls might return
undef.

see commnets below:

> ---
>  PVE/Service/pveproxy.pm       | 21 ++++++++++++
>  www/index.html.tpl            |  2 +-
>  www/manager6/UIOptions.js     |  6 ++++
>  www/manager6/dc/OptionView.js | 61 +++++++++++++++++++++++++++++++++++
>  4 files changed, 89 insertions(+), 1 deletion(-)
>
> diff --git a/PVE/Service/pveproxy.pm b/PVE/Service/pveproxy.pm
> index c6011a00..811295f4 100755
> --- a/PVE/Service/pveproxy.pm
> +++ b/PVE/Service/pveproxy.pm
> @@ -202,6 +202,24 @@ my sub get_path_mtime {
>      return $mtime;
>  }
>  
> +# builds the window title from the datacenter.cfg configuration
> +my $get_window_title = sub {
> +    my ($title_enum, $nodename) = @_;
> +
> +    my $title = "$nodename";
> +
> +    if ($title_enum eq "node-and-cluster") {
> +        my $clinfo = PVE::Cluster::get_clinfo();
> +        my $clustername = $clinfo->{cluster}->{name};
			 	^^^^
This returned undef once when i was messing with the cluster settings, when i reloaded the web page using CTL +
F5 on firefox. This resulted in my window title to
default to the link address of the web page. I suspect this is because
the error is surpressed somewhere, probably by the eval block below.

The correct formatting appeared when i accessed the WebUI in a new
brower tab.

Initializing the window title using a safe default in case of
an error, might be an option.
> +
> +        $title = "$nodename - $clustername";
> +    } elsif ($title_enum eq "fqdn") {
> +        $title = PVE::Tools::get_fqdn();
			^^^^^
PVE:Tools::get_fqdn requires a parameter. The first line of the function
(on my test machine is):
my ($nodename) = @_;

This resulted in an error (captured by journalctl -u pveproxy -f):
Use of uninitialized value $lang in concatenation (.) or string at /usr/share/perl5/PVE/Service/pveproxy.pm line 273.
pve-3 pveproxy[27666]: getaddrinfo: Name or service not known at /usr/share/perl5/PVE/Tools.pm line 915.

I belive the fix is:
$title = PVE::Tools::get_fqdn($nodename);

Also: a misconfigured /etc/hosts file might result in the function
returning undef --> some type of error handling is needed.
> +    }
> +
> +    return $title . " - Proxmox Virtual Environment";
> +};
> +
>  # NOTE: Requests to those pages are not authenticated so we must be very careful here
>  sub get_index {
>      my ($nodename, $server, $r, $args) = @_;
> @@ -232,10 +250,12 @@ sub get_index {
>          }
>      }
>  
> +    my $window_title;
>      my $consent_text;
>      eval {
>          my $dc_conf = PVE::Cluster::cfs_read_file('datacenter.cfg');
>          $consent_text = $dc_conf->{'consent-text'};
> +        $window_title = $get_window_title->($dc_conf->{'ui-settings'}->{'title'}, $nodename);
>  
>          if (!$lang) {
>              $lang = $dc_conf->{language} // 'en';
> @@ -276,6 +296,7 @@ sub get_index {
>          token => $token,
>          console => $args->{console},
>          nodename => $nodename,
> +        window_title => $window_title,
>          debug => $debug,
>          version => "$version",
>          wtversion => $wtversion,
> diff --git a/www/index.html.tpl b/www/index.html.tpl
> index 74ee02d9..5138b7cc 100644
> --- a/www/index.html.tpl
> +++ b/www/index.html.tpl
> @@ -4,7 +4,7 @@
>      <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
>      <meta http-equiv="X-UA-Compatible" content="IE=edge">
>      <meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1, user-scalable=no">
> -    <title>[% nodename %] - Proxmox Virtual Environment</title>
> +    <title>[% window_title %]</title>
>      <link rel="icon" sizes="128x128" href="/pve2/images/logo-128.png" />
>      <link rel="apple-touch-icon" sizes="128x128" href="/pve2/images/logo-128.png" />
>      <link rel="stylesheet" type="text/css" href="/pve2/ext6/theme-crisp/resources/theme-crisp-all.css?ver=7.0.0" />
> diff --git a/www/manager6/UIOptions.js b/www/manager6/UIOptions.js
> index 8c4674af..bb0af8d5 100644
> --- a/www/manager6/UIOptions.js
> +++ b/www/manager6/UIOptions.js
> @@ -90,6 +90,12 @@ Ext.define('PVE.UIOptions', {
>          alphabetical: gettext('Alphabetical'),
>      },
>  
> +    titleOptions: {
> +        __default__: '$nodename - Proxmox Virtual Environment',
> +        'node-and-cluster': '$nodename - $clustername - Proxmox Virtual Environment',
> +        fqdn: '$fqdn - Proxmox Virtual Environment',
> +    },
> +
>      shouldSortTags: function () {
>          return !(PVE.UIOptions.options['tag-style']?.ordering === 'config');
>      },
> diff --git a/www/manager6/dc/OptionView.js b/www/manager6/dc/OptionView.js
> index dc12aa7e..208cc147 100644
> --- a/www/manager6/dc/OptionView.js
> +++ b/www/manager6/dc/OptionView.js
> @@ -91,6 +91,67 @@ Ext.define('PVE.dc.OptionView', {
>              defaultValue: '__default__',
>              deleteEmpty: true,
>          });
> +        me.rows['ui-settings'] = {
> +            required: true,
> +            renderer: (value) => {
> +                if (value === undefined) {
> +                    return gettext('No Overrides');
> +                }
> +                let txt = '';
> +                if (value.title) {
> +                    txt += Ext.String.format(gettext('Title: {0}'), value.title);
> +                }
> +                return txt;
> +            },
> +            header: gettext('Ui Settings'),
> +            editor: {
> +                xtype: 'proxmoxWindowEdit',
> +                width: 800,
> +                subject: gettext('Ui Settings'),
> +                fieldDefaults: {
> +                    labelWidth: 100,
> +                },
> +                url: '/api2/extjs/cluster/options',
> +                items: [
> +                    {
> +                        xtype: 'inputpanel',
> +                        setValues: function (values) {
> +                            if (values === undefined) {
> +                                return undefined;
> +                            }
> +                            values = values?.['ui-settings'] ?? {};
> +                            values.title = values.title || '__default__';
> +                            return Proxmox.panel.InputPanel.prototype.setValues.call(this, values);
> +                        },
> +                        onGetValues: function (values) {
> +                            let style = {};
> +                            if (values.title) {
> +                                style.title = values.title;
> +                            }
> +                            let value = PVE.Parser.printPropertyString(style);
> +                            if (value === '') {
> +                                return {
> +                                    delete: 'ui-settings',
> +                                };
> +                            }
> +                            return {
> +                                'ui-settings': value,
> +                            };
> +                        },
> +                        items: [
> +                            {
> +                                xtype: 'proxmoxKVComboBox',
> +                                name: 'title',
> +                                fieldLabel: gettext('Title'),
> +                                comboItems: Object.entries(PVE.UIOptions.titleOptions),
> +                                deleteEmpty: true,
> +                                defaultValue: '__default__',
> +                            },
> +                        ],
> +                    },
> +                ],
> +            },
> +        };
>          me.add_text_row('email_from', gettext('Email from address'), {
>              deleteEmpty: true,
>              vtype: 'proxmoxMail',

Re: [PATCH cluster v2 2/2] fix #5475: configurable window title

[Elias Huhsovitz]

The new setting matches the existing format well.
See 1 nit below. 

That being said:

Reviewed-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
Tested-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>

On Mon Jul 20, 2026 at 12:03 PM CEST, Thomas Ellmenreich wrote:
> Added a new 'ui-settings' format string to the datacenter.cfg. Currently
> only contains a 'title' option to determine the window title of the
> nodes browser tab.
>
> Signed-off-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
> ---
>  src/PVE/DataCenterConfig.pm | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/src/PVE/DataCenterConfig.pm b/src/PVE/DataCenterConfig.pm
> index 004122e..751d8ed 100644
> --- a/src/PVE/DataCenterConfig.pm
> +++ b/src/PVE/DataCenterConfig.pm
> @@ -344,6 +344,16 @@ my $user_tag_privs_format = {
>      },
>  };
>  
> +my $ui_settings_format = {
> +    'title' => {
nit: The 'title' in the ui_settings_format might be a bit vagaue, since the
format might be used in future UI settings. Perhaps some thing like
'page-title' or 'tab-title' might be better suited. But this is very
pedantic of me.
> +        optional => 1,
> +        type => 'string',
> +        enum => ['node-and-cluster', 'fqdn'],
> +        description =>
> +            "Define the text displayed in the browser tab title for all nodes in this cluster.",
> +    },
> +};
> +
>  my $datacenter_schema = {
>      type => "object",
>      additionalProperties => 0,
> @@ -436,6 +446,12 @@ my $datacenter_schema = {
>              # FIXME: remove 'applet' with 9.0 (add pve8to9 check!)
>              enum => ['applet', 'vv', 'html5', 'xtermjs'],
>          },
> +        'ui-settings' => {
> +            optional => 1,
> +            type => "string",
> +            format => $ui_settings_format,
> +            description => "For cluster wide ui settings.",
> +        },
>          email_from => {
>              optional => 1,
>              type => 'string',
> @@ -618,6 +634,10 @@ sub parse_datacenter_config {
>          $res->{'tag-style'} = parse_property_string($tag_style_format, $tag_style);
>      }
>  
> +    if (my $ui_settings = $res->{'ui-settings'}) {
> +        $res->{'ui-settings'} = parse_property_string($ui_settings_format, $ui_settings);
> +    }
> +
>      if (my $user_tag_privs = $res->{'user-tag-access'}) {
>          $res->{'user-tag-access'} = parse_property_string($user_tag_privs_format, $user_tag_privs);
>  
> @@ -711,6 +731,11 @@ sub write_datacenter_config {
>          $cfg->{'tag-style'} = PVE::JSONSchema::print_property_string($tag_style, $tag_style_format);
>      }
>  
> +    if (ref(my $ui_settings = $cfg->{'ui-settings'})) {
> +        $cfg->{'ui-settings'} =
> +            PVE::JSONSchema::print_property_string($ui_settings, $ui_settings_format);
> +    }
> +
>      if (ref(my $user_tag_privs = $cfg->{'user-tag-access'})) {
>          if (my $user_tags = $user_tag_privs->{'user-allow-list'}) {
>              $user_tag_privs->{'user-allow-list'} = join(';', sort $user_tags->@*);

Re: [PATCH cluster v2 2/2] fix #5475: configurable window title

[Thomas Ellmenreich]

Thanks for your review, I'm addressing the issues on the other patch
right now!

On Wed Jul 22, 2026 at 12:38 PM CEST, Elias Huhsovitz wrote:

[snip]

>> +my $ui_settings_format = {
>> +    'title' => {
> nit: The 'title' in the ui_settings_format might be a bit vagaue, since the
> format might be used in future UI settings. Perhaps some thing like
> 'page-title' or 'tab-title' might be better suited. But this is very
> pedantic of me.

For this one, I had it as 'window-title' before, but with the suggestion
to create the 'ui-settings' format string, @Thomas Lamprecht [1] also
mentioned changing the 'window-title' name to just 'title' which I
found fitting.

[1]: https://lore.proxmox.com/pve-devel/3f04e2b9-d39f-4604-b169-c3ae530256af@proxmox.com/

[snip]

Re: [PATCH manager v2 1/2] fix #5475: configurable window title

[Thomas Ellmenreich]

Thanks again for your review. I realised I had a question here as well.

[snip]

> 1. The setting names render including the $ symbol, e.g. ($nodename, $fqdn) in the WebUI. 
> This makes it seem broken from an end user point of view. 
> You generally do not expect placeholder values in settings menu.

Rendering the '$' was on purpose, as they are placeholders. Would you
recommend removing just the '$' or would you rather render the actual
values?

I technically copied it from somewhere, as I had a different placeholder
marker, although I can't seem to find that reference anymore.

> 2. I encountered a bug regarding the fdqn setting (see comment below)

I'm investigating that, especially why I didn't have problems with it.
Thanks for noticing.

> 3. I recommend adding some general robustness in case of undef values:
> Initialize $window_title to a safe value not relying on the cluster, in
> case of an error. Expect that the underlying API/calls might return
> undef.

It was an active choice to show something broken in the case of undef, as I
thought hiding the error could also be confusing. But then again, I don't mind
using the fallback value.

Re: [PATCH manager v2 1/2] fix #5475: configurable window title

[Elias Huhsovitz]

A few comments adressing 1. & 3.

On Wed Jul 22, 2026 at 2:43 PM CEST, Thomas Ellmenreich wrote:
> Thanks again for your review. I realised I had a question here as well.
>
> [snip]
>
>> 1. The setting names render including the $ symbol, e.g. ($nodename, $fqdn) in the WebUI. 
>> This makes it seem broken from an end user point of view. 
>> You generally do not expect placeholder values in settings menu.
>
> Rendering the '$' was on purpose, as they are placeholders. Would you
> recommend removing just the '$' or would you rather render the actual
> values?
>
> I technically copied it from somewhere, as I had a different placeholder
> marker, although I can't seem to find that reference anymore.

While using $ is a standard programming convention for variables, 
displaying it literally in a user-facing dropdown 
(e.g., $nodename - Proxmox Virtual Environment) 
can look like a broken string interpolation or a rendering bug to an end user. 

Instead of showing the template string, my suggestions would be
descriptive labels for the items, e.g.:
* __default__ -> "Node Name (Default)"
* node-and-cluster -> "Node and Cluster Name"
* fqdn -> Fully Qualified Domain Name (FQDN)

>> 3. I recommend adding some general robustness in case of undef values:
>> Initialize $window_title to a safe value not relying on the cluster, in
>> case of an error. Expect that the underlying API/calls might return
>> undef.
>
> It was an active choice to show something broken in the case of undef, as I
> thought hiding the error could also be confusing. But then again, I don't mind
> using the fallback value.

IMO the problem is that the undef might get dereference and cause a 'die' 
potentially aborting the entire entire eval block in pveproy.pm:255.

I would advocate for "graceful" degradation:
- add a defined check to prevent the crash
- initialize the $window_title with a safe default.

You can also add a warn statement inside the subroutine so it does not
fail silently.