From: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
To: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
Cc: pve-devel@lists.proxmox.com
Subject: Re: [PATCH manager v4 1/2] fix #5475: configurable window title
Date: Wed, 16 Sep 2026 16:51:05 +0200 [thread overview]
Message-ID: <wwvnuxg2padjlnxbzjgq6u7ur2if4yx5zazox45oyrinphblzu@uevfbnmvhdfw> (raw)
In-Reply-To: <20260914062202.27182-2-t.ellmenreich@proxmox.com>
thanks for the patch! I made a few comments inline,
I also noticed that setting the window title to node-and-cluster fails
silently on standalone nodes, which took me a bit to realize. not sure
if rejecting it for standalone nodes at the API layer makes sense since
the setting becomes meaningful once a cluster is created, but maybe a
hint in the dialog would be enough?
On Mon, Sep 14, 2026 at 08:22:01AM +0200, Thomas Ellmenreich wrote:
> Added a new 'ui-settings' format string to the datacenter.cfg which now
> has a 'title' option to configure the browser tab title for pve tabs.
>
> Adjusted the index.html templating to construct the correct window title
> on every request according to the cluster configuration.
>
> Signed-off-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
> ---
> PVE/Service/pveproxy.pm | 27 ++++++++++++++++
> www/index.html.tpl | 2 +-
> www/manager6/UIOptions.js | 6 ++++
> www/manager6/dc/OptionView.js | 61 +++++++++++++++++++++++++++++++++++
> 4 files changed, 95 insertions(+), 1 deletion(-)
>
> diff --git a/PVE/Service/pveproxy.pm b/PVE/Service/pveproxy.pm
> index dfdd014c..d0ebdb71 100755
> --- a/PVE/Service/pveproxy.pm
> +++ b/PVE/Service/pveproxy.pm
> @@ -202,6 +202,30 @@ my sub get_path_mtime {
> return $mtime;
> }
>
> +# builds the window title from the datacenter.cfg configuration
> +my $get_window_title = sub {
nit: `my sub foo` seems to be preferred over `my $foo = sub` in this
file.
> + my ($title_enum, $nodename) = @_;
> +
> + my $title;
> +
> + if (defined $title_enum) {
> + eval {
> + if ($title_enum eq "node-and-cluster") {
> + my $clinfo = PVE::Cluster::get_clinfo();
> + my $clustername = $clinfo->{cluster}->{name};
> +
> + $title = "$nodename - $clustername" if $clustername;
> + } elsif ($title_enum eq "fqdn") {
> + $title = PVE::Tools::get_fqdn($nodename);
haven't looked too deeply into it, but afaict on a standard setup this
resolves the FQDN from /etc/hosts. still, this lookup is now reachable
from an unauthenticated path, could that be problematic in some setups?
not sure how likely a DNS stall is here, just wanted to mention it.
> + }
> + };
> +
> + warn "failed to create '$title_enum' window title: $@" if $@;
> + }
> +
> + return ($title // "$nodename") . " - 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 +256,12 @@ sub get_index {
> }
> }
>
> + my $window_title;
currently if anything dies in the eval block before $get_window_title is
called, $window_title stays undef.
you coudl read only the title setting inside the eval and call
$get_window_title after the block, since it cannot die anyway.
> 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';
> @@ -277,6 +303,7 @@ sub get_index {
> console => $args->{console},
> nodename => $nodename,
> arch => PVE::Tools::get_host_dpkg_arch(),
> + window_title => $window_title,
> debug => $debug,
> version => "$version",
> wtversion => $wtversion,
> diff --git a/www/index.html.tpl b/www/index.html.tpl
> index c18e6411..a45bad39 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..49307fa4 100644
> --- a/www/manager6/UIOptions.js
> +++ b/www/manager6/UIOptions.js
> @@ -90,6 +90,12 @@ Ext.define('PVE.UIOptions', {
> alphabetical: gettext('Alphabetical'),
> },
>
> + titleOptions: {
> + __default__: 'Node Name (Default)',
> + 'node-and-cluster': 'Node and Cluster Name',
> + fqdn: 'Fully Qualified Domain Name (FQDN)',
> + },
these should probably also be wrapped in gettext
> +
> 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);
nit: this shows the raw enum value, which confused me a bit given the
dropdown in the edit window shows the display text. as you noted
off-list, the display names could make the column quite a lot wider,
however I don't think this would be an issue, since there are other
columns that are pretty wide as well (e.g. the tag style overrides).
> + }
> + return txt;
> + },
> + header: gettext('Ui Settings'),
as far as I could tell from a quick search through the codebase, we do
not use Ui (with small i) anywhere in user-facing strings, I think UI
would be better here.
> + editor: {
> + xtype: 'proxmoxWindowEdit',
> + width: 800,
> + subject: gettext('Ui Settings'),
same here
> + 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
>
>
>
>
>
next prev parent reply other threads:[~2026-09-16 14:51 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-14 6:22 [PATCH cluster/manager v4 0/2] Configurable window titles for nodes Thomas Ellmenreich
2026-09-14 6:22 ` [PATCH manager v4 1/2] fix #5475: configurable window title Thomas Ellmenreich
2026-09-16 14:51 ` Arthur Bied-Charreton [this message]
2026-09-14 6:22 ` [PATCH cluster v4 2/2] " Thomas Ellmenreich
2026-09-16 14:52 ` Arthur Bied-Charreton
2026-09-16 15:02 ` [PATCH cluster/manager v4 0/2] Configurable window titles for nodes Arthur Bied-Charreton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=wwvnuxg2padjlnxbzjgq6u7ur2if4yx5zazox45oyrinphblzu@uevfbnmvhdfw \
--to=a.bied-charreton@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
--cc=t.ellmenreich@proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox