From 22dc5ea1b5089ec2d1f7ab5006741639ff1b1d13 Mon Sep 17 00:00:00 2001 From: Tom Elliott Date: Sat, 5 Sep 2026 09:08:46 -0500 Subject: [PATCH] Confirm a quick task in a modal, not window.confirm() The one-click Deploy/Capture/Multi-Cast buttons in the info card asked for confirmation through window.confirm(). It works, and it looks like nothing else in FOG: the browser dialog cannot be styled, ignores the dark theme, and prefixes the page URL, so the one place the app asks before wiping a machine is the one place that reads as though the site got something wrong. renderQuickTaskActions() now emits a modal beside the buttons, the same shape assocDelModal() uses -- which is what every other "are you sure" in this app already looks like. One modal per card, not one per button: the script fills its body from the clicked button's data-confirm, so two buttons cannot drift into two wordings of the same question. The text is still built server side and still translated. A .modal is position:fixed and display:none until shown, so it contributes nothing to the flex row it is emitted into. In fog.common.js the click handler now records which button opened the modal and the request moves to fire(), called from the modal's Create. `pending` is cleared before the request, so a second click during the hide animation has nothing left to commit, and the per-button in-flight lock is unchanged. Both handlers share the click.fogQuickTask namespace so the existing .off() still clears the pair on AJAX nav. The body is set with .text(), never .html() -- data-confirm carries an admin-supplied host or group name. Measured in both themes with the real stylesheets: the buttons stay at 4.69:1, the modal's Cancel at 5.92:1 light / 11.85:1 dark, and its Create (the modal-warning fill) at 5.14:1. All pass WCAG AA. FOG_BCACHE_VER 361 -> 362, since fog.common.js changed. tests/info-card-quick-tasks.test.php grows five checks covering the modal: that it is emitted, that it is the only one, that the footer is a dismiss plus a commit, that the body is a filled-in placeholder rather than static text, and that no window.confirm() is left in the handler. Each was proven by reintroducing the defect and watching it go red. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JWJMQYE2br8E7Ehr55SJp2 --- packages/web/management/js/fog/fog.common.js | 82 ++++++++++++------- .../de_DE.UTF-8/LC_MESSAGES/messages.po | 5 +- .../en_US.UTF-8/LC_MESSAGES/messages.po | 5 +- .../es_ES.UTF-8/LC_MESSAGES/messages.po | 5 +- .../eu_ES.UTF-8/LC_MESSAGES/messages.po | 5 +- .../fr_FR.UTF-8/LC_MESSAGES/messages.po | 5 +- .../it_IT.UTF-8/LC_MESSAGES/messages.po | 5 +- .../ja_JP.UTF-8/LC_MESSAGES/messages.po | 5 +- .../web/management/languages/messages.pot | 4 +- .../pt_BR.UTF-8/LC_MESSAGES/messages.po | 5 +- .../zh_CN.UTF-8/LC_MESSAGES/messages.po | 5 +- packages/web/src/Base/FOGPageRender.php | 38 ++++++++- packages/web/src/Base/System.php | 2 +- tests/info-card-quick-tasks.test.php | 44 +++++++++- 14 files changed, 171 insertions(+), 44 deletions(-) diff --git a/packages/web/management/js/fog/fog.common.js b/packages/web/management/js/fog/fog.common.js index 1a04a1b757..5d2d29e156 100644 --- a/packages/web/management/js/fog/fog.common.js +++ b/packages/web/management/js/fog/fog.common.js @@ -5417,6 +5417,12 @@ function setupInfoCard() { * clicking a host name, and one stray click would deploy over a running * machine -- or, from a group, over all of them. The text is built server * side (FOGPageRender::renderQuickTaskActions) because it is translated. + * + * The confirmation is the page's own modal rather than window.confirm(): the + * browser dialog cannot be styled, ignores the dark theme, and prefixes the + * page URL, so it reads as something outside the application. Every button + * shares the one modal, which carries the clicked button's data-confirm as + * its body and remembers which button opened it. */ function setupInfoCardActions() { // Guards the window between the click and the server's answer. Per button @@ -5427,39 +5433,59 @@ function setupInfoCardActions() { // otherwise be two identical taskings. var running = {}; - // Delegated and namespaced: the card is torn down and rebuilt with the - // page on every AJAX nav, so a direct binding would be lost on the first - // one and doPageLoad() would stack a new one per visit. + // Which button opened the modal. Cleared on every open so a dismissed + // confirmation cannot be committed by the next one. + var pending = null; + + function fire(btn) { + var node = btn.data('node'), + id = btn.data('id'), + type = btn.data('type'), + key = node + ':' + id + ':' + type; + if (running[key]) { + return; + } + running[key] = true; + $.apiCall( + 'post', + '../management/index.php?node=' + encodeURIComponent(node) + + '&sub=deploy&id=' + encodeURIComponent(id) + + '&type=' + encodeURIComponent(type), + {scheduleType: 'instant'}, + function() { + // Both outcomes land here and both only need the lock released. + // apiCall has already drawn the toast, and there is nothing on + // this page to repaint: the card's Last Deployed is the date the + // last task FINISHED, which a task just queued has not. + running[key] = false; + } + ); + } + + // Delegated and namespaced: the card and its modal are torn down and + // rebuilt with the page on every AJAX nav, so a direct binding would be + // lost on the first one and doPageLoad() would stack a new one per visit. + // Both handlers share the one namespace so the .off() clears the pair. $(document) .off('click.fogQuickTask') .on('click.fogQuickTask', '.fog-quicktask', function(e) { e.preventDefault(); - var btn = $(this), - node = btn.data('node'), - id = btn.data('id'), - type = btn.data('type'), - key = node + ':' + id + ':' + type; - if (running[key]) { - return; - } - if (!window.confirm(btn.data('confirm'))) { - return; + pending = $(this); + // .text(), never .html(): data-confirm carries a host or group name, + // which is admin-supplied. + $('#quicktask-confirm-text').text(pending.data('confirm')); + $('#quicktask-confirm-modal').modal('show'); + }) + .on('click.fogQuickTask', '#quicktask-confirm-go', function(e) { + e.preventDefault(); + var btn = pending; + // Taken before the request so a second click during the hide + // animation has nothing left to commit. + pending = null; + $('#quicktask-confirm-modal').modal('hide'); + if (btn) { + fire(btn); } - running[key] = true; - $.apiCall( - 'post', - '../management/index.php?node=' + encodeURIComponent(node) - + '&sub=deploy&id=' + encodeURIComponent(id) - + '&type=' + encodeURIComponent(type), - {scheduleType: 'instant'}, - function() { - // Both outcomes land here and both only need the lock released. - // apiCall has already drawn the toast, and there is nothing on - // this page to repaint: the card's Last Deployed is the date the - // last task FINISHED, which a task just queued has not. - running[key] = false; - } - ); }); } diff --git a/packages/web/management/languages/de_DE.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/de_DE.UTF-8/LC_MESSAGES/messages.po index 40cc95fc47..d47ed9c2a9 100644 --- a/packages/web/management/languages/de_DE.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/de_DE.UTF-8/LC_MESSAGES/messages.po @@ -2437,6 +2437,10 @@ msgstr "Benutzer erstellen fehlgeschlagen" msgid "Create task form success" msgstr "Benutzer erfolgreich erstellt" +#, fuzzy +msgid "Create tasking" +msgstr "Neues Snapin erstellen" + msgid "Create tasking succeeded" msgstr "" @@ -10420,7 +10424,6 @@ msgstr "" msgid "The term goes in q. limit caps results PER CLASS, not overall, and this route is not paged -- there is no nextUrl to follow. Within a class, names that start with the term sort first. Both fields may also be sent as POST body fields. Also reachable as /search." msgstr "" -#, php-format msgid "The term. Because it is a path segment, a term containing / ? # or % cannot travel this way; use /unisearch?q= instead." msgstr "" diff --git a/packages/web/management/languages/en_US.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/en_US.UTF-8/LC_MESSAGES/messages.po index 1681a995ed..3d3c730a0c 100644 --- a/packages/web/management/languages/en_US.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/en_US.UTF-8/LC_MESSAGES/messages.po @@ -2440,6 +2440,10 @@ msgstr "User created" msgid "Create task form success" msgstr "User created" +#, fuzzy +msgid "Create tasking" +msgstr "Create New %s" + msgid "Create tasking succeeded" msgstr "" @@ -10429,7 +10433,6 @@ msgstr "" msgid "The term goes in q. limit caps results PER CLASS, not overall, and this route is not paged -- there is no nextUrl to follow. Within a class, names that start with the term sort first. Both fields may also be sent as POST body fields. Also reachable as /search." msgstr "" -#, php-format msgid "The term. Because it is a path segment, a term containing / ? # or % cannot travel this way; use /unisearch?q= instead." msgstr "" diff --git a/packages/web/management/languages/es_ES.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/es_ES.UTF-8/LC_MESSAGES/messages.po index 3ee7b9f6e8..0d18efc9d5 100644 --- a/packages/web/management/languages/es_ES.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/es_ES.UTF-8/LC_MESSAGES/messages.po @@ -2458,6 +2458,10 @@ msgstr "creado por el usuario" msgid "Create task form success" msgstr "creado por el usuario" +#, fuzzy +msgid "Create tasking" +msgstr "Crear nuevo grupo" + msgid "Create tasking succeeded" msgstr "" @@ -10587,7 +10591,6 @@ msgstr "" msgid "The term goes in q. limit caps results PER CLASS, not overall, and this route is not paged -- there is no nextUrl to follow. Within a class, names that start with the term sort first. Both fields may also be sent as POST body fields. Also reachable as /search." msgstr "" -#, php-format msgid "The term. Because it is a path segment, a term containing / ? # or % cannot travel this way; use /unisearch?q= instead." msgstr "" diff --git a/packages/web/management/languages/eu_ES.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/eu_ES.UTF-8/LC_MESSAGES/messages.po index e0fc0180d9..c7e7dd915b 100644 --- a/packages/web/management/languages/eu_ES.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/eu_ES.UTF-8/LC_MESSAGES/messages.po @@ -2437,6 +2437,10 @@ msgstr "Benutzer erstellen fehlgeschlagen" msgid "Create task form success" msgstr "Benutzer erfolgreich erstellt" +#, fuzzy +msgid "Create tasking" +msgstr "Neues Snapin erstellen" + msgid "Create tasking succeeded" msgstr "" @@ -10421,7 +10425,6 @@ msgstr "" msgid "The term goes in q. limit caps results PER CLASS, not overall, and this route is not paged -- there is no nextUrl to follow. Within a class, names that start with the term sort first. Both fields may also be sent as POST body fields. Also reachable as /search." msgstr "" -#, php-format msgid "The term. Because it is a path segment, a term containing / ? # or % cannot travel this way; use /unisearch?q= instead." msgstr "" diff --git a/packages/web/management/languages/fr_FR.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/fr_FR.UTF-8/LC_MESSAGES/messages.po index 3b00b8e97a..2ced59edc7 100644 --- a/packages/web/management/languages/fr_FR.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/fr_FR.UTF-8/LC_MESSAGES/messages.po @@ -2440,6 +2440,10 @@ msgstr "utilisateur créé" msgid "Create task form success" msgstr "utilisateur créé" +#, fuzzy +msgid "Create tasking" +msgstr "Créer un nouveau %s" + msgid "Create tasking succeeded" msgstr "" @@ -10413,7 +10417,6 @@ msgstr "" msgid "The term goes in q. limit caps results PER CLASS, not overall, and this route is not paged -- there is no nextUrl to follow. Within a class, names that start with the term sort first. Both fields may also be sent as POST body fields. Also reachable as /search." msgstr "" -#, php-format msgid "The term. Because it is a path segment, a term containing / ? # or % cannot travel this way; use /unisearch?q= instead." msgstr "" diff --git a/packages/web/management/languages/it_IT.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/it_IT.UTF-8/LC_MESSAGES/messages.po index a204c10efa..1118ae0608 100644 --- a/packages/web/management/languages/it_IT.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/it_IT.UTF-8/LC_MESSAGES/messages.po @@ -2382,6 +2382,10 @@ msgstr "Creazione utente fallita" msgid "Create task form success" msgstr "Creazione utente riuscita" +#, fuzzy +msgid "Create tasking" +msgstr "Crea nuovo snapin" + msgid "Create tasking succeeded" msgstr "" @@ -10136,7 +10140,6 @@ msgstr "" msgid "The term goes in q. limit caps results PER CLASS, not overall, and this route is not paged -- there is no nextUrl to follow. Within a class, names that start with the term sort first. Both fields may also be sent as POST body fields. Also reachable as /search." msgstr "" -#, php-format msgid "The term. Because it is a path segment, a term containing / ? # or % cannot travel this way; use /unisearch?q= instead." msgstr "" diff --git a/packages/web/management/languages/ja_JP.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/ja_JP.UTF-8/LC_MESSAGES/messages.po index ed897e43b5..7fa50c2fc5 100644 --- a/packages/web/management/languages/ja_JP.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/ja_JP.UTF-8/LC_MESSAGES/messages.po @@ -2367,6 +2367,10 @@ msgstr "タスク作成対象" msgid "Create task form success" msgstr "タスク状態を作成" +#, fuzzy +msgid "Create tasking" +msgstr "新しいスナップインを作成" + #, fuzzy msgid "Create tasking succeeded" msgstr "タスク状態を作成" @@ -10093,7 +10097,6 @@ msgstr "" msgid "The term goes in q. limit caps results PER CLASS, not overall, and this route is not paged -- there is no nextUrl to follow. Within a class, names that start with the term sort first. Both fields may also be sent as POST body fields. Also reachable as /search." msgstr "" -#, php-format msgid "The term. Because it is a path segment, a term containing / ? # or % cannot travel this way; use /unisearch?q= instead." msgstr "" diff --git a/packages/web/management/languages/messages.pot b/packages/web/management/languages/messages.pot index 660d09f7f0..93bf06b48e 100644 --- a/packages/web/management/languages/messages.pot +++ b/packages/web/management/languages/messages.pot @@ -2106,6 +2106,9 @@ msgstr "" msgid "Create task form success" msgstr "" +msgid "Create tasking" +msgstr "" + msgid "Create tasking succeeded" msgstr "" @@ -8932,7 +8935,6 @@ msgstr "" msgid "The term goes in q. limit caps results PER CLASS, not overall, and this route is not paged -- there is no nextUrl to follow. Within a class, names that start with the term sort first. Both fields may also be sent as POST body fields. Also reachable as /search." msgstr "" -#, php-format msgid "The term. Because it is a path segment, a term containing / ? # or % cannot travel this way; use /unisearch?q= instead." msgstr "" diff --git a/packages/web/management/languages/pt_BR.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/pt_BR.UTF-8/LC_MESSAGES/messages.po index 101f14bfb6..fcb06dc94a 100644 --- a/packages/web/management/languages/pt_BR.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/pt_BR.UTF-8/LC_MESSAGES/messages.po @@ -2440,6 +2440,10 @@ msgstr "usuário criado" msgid "Create task form success" msgstr "usuário criado" +#, fuzzy +msgid "Create tasking" +msgstr "Criar novo %s" + msgid "Create tasking succeeded" msgstr "" @@ -10416,7 +10420,6 @@ msgstr "" msgid "The term goes in q. limit caps results PER CLASS, not overall, and this route is not paged -- there is no nextUrl to follow. Within a class, names that start with the term sort first. Both fields may also be sent as POST body fields. Also reachable as /search." msgstr "" -#, php-format msgid "The term. Because it is a path segment, a term containing / ? # or % cannot travel this way; use /unisearch?q= instead." msgstr "" diff --git a/packages/web/management/languages/zh_CN.UTF-8/LC_MESSAGES/messages.po b/packages/web/management/languages/zh_CN.UTF-8/LC_MESSAGES/messages.po index 07863ee01c..a80e973b10 100644 --- a/packages/web/management/languages/zh_CN.UTF-8/LC_MESSAGES/messages.po +++ b/packages/web/management/languages/zh_CN.UTF-8/LC_MESSAGES/messages.po @@ -2440,6 +2440,10 @@ msgstr "用户创建" msgid "Create task form success" msgstr "用户创建" +#, fuzzy +msgid "Create tasking" +msgstr "新建%s" + msgid "Create tasking succeeded" msgstr "" @@ -10416,7 +10420,6 @@ msgstr "" msgid "The term goes in q. limit caps results PER CLASS, not overall, and this route is not paged -- there is no nextUrl to follow. Within a class, names that start with the term sort first. Both fields may also be sent as POST body fields. Also reachable as /search." msgstr "" -#, php-format msgid "The term. Because it is a path segment, a term containing / ? # or % cannot travel this way; use /unisearch?q= instead." msgstr "" diff --git a/packages/web/src/Base/FOGPageRender.php b/packages/web/src/Base/FOGPageRender.php index 01b5f320da..ac3bfc334f 100644 --- a/packages/web/src/Base/FOGPageRender.php +++ b/packages/web/src/Base/FOGPageRender.php @@ -566,7 +566,7 @@ protected static function noteSourceAttrs($source) * commit action -- the General tab's Update is -- so these are two * shortcuts in a header strip, not a decision cluster in a form footer, * and the weight a red button would carry is carried by the - * confirmation instead. It is also what the list grid's own quick + * confirmation modal instead. It is also what the list grid's own quick * buttons are, since DataTables draws its button bar that way. * * Filled, NOT btn-outline-secondary, and that is a contrast decision @@ -636,9 +636,43 @@ public static function renderQuickTaskActions( return ''; } + // The confirmation is a modal, not window.confirm(). The browser's + // own dialog cannot be styled, ignores the dark theme entirely, and + // announces itself with the page's URL -- next to AdminLTE it reads + // as something the site got wrong. Same shape as assocDelModal(), + // which is what every other "are you sure" in this app looks like. + // + // ONE modal for the whole card, filled in by the script from the + // clicked button's data-confirm, rather than one per button: two + // would eventually say the same thing in two different wordings. + // + // Emitted next to the buttons, the way assocDelModal() sits in its + // card-footer. A .modal is position:fixed and display:none until + // shown, so it adds nothing to the flex row it nominally lives in. + $modal = self::makeModal( + 'quicktask-confirm-modal', + '

' . \Initiator::e(_('Create tasking')) + . '

', + '

', + self::makeButton( + 'quicktask-confirm-cancel', + _('Cancel'), + 'btn btn-outline-secondary float-start', + 'type="button" data-bs-dismiss="modal"' + ) + . self::makeButton( + 'quicktask-confirm-go', + _('Create'), + 'btn btn-outline-secondary float-end', + 'type="button"' + ), + '', + 'warning' + ); + return '
' . $buttons . '
'; + . '">' . $buttons . '' . $modal; } protected function renderInfoCard() diff --git a/packages/web/src/Base/System.php b/packages/web/src/Base/System.php index d055b817ef..cb3bc95eb2 100644 --- a/packages/web/src/Base/System.php +++ b/packages/web/src/Base/System.php @@ -131,7 +131,7 @@ public function __construct() // permanently "up to date" from the updater's point of view and will // never run another indexed step, whatever this constant says. define('FOG_SCHEMA', 430); - define('FOG_BCACHE_VER', 361); + define('FOG_BCACHE_VER', 362); define('FOG_CLIENT_VERSION', '0.13.0'); // GH-959: iPXE lives in FOGProject/fog-ipxe and its binaries arrive as // a release asset. Pinned here rather than tracked as "latest" so a diff --git a/tests/info-card-quick-tasks.test.php b/tests/info-card-quick-tasks.test.php index 844f9fcf8f..c4a3939f2b 100644 --- a/tests/info-card-quick-tasks.test.php +++ b/tests/info-card-quick-tasks.test.php @@ -303,10 +303,48 @@ static function ($b) { // body-sized text. Filled puts white on #6c757d and holds 4.69:1 in both // themes. Measured in a browser against the shipped stylesheets; pinned here // as the class, because the class is the part a future edit would change. +// +// Scoped to the BUTTON GROUP, not the whole markup: the confirmation modal +// that ships alongside it is deliberately outline-secondary, because that is +// what every other confirm modal in the app uses and a modal's footer is not +// a header strip. Asserting over the whole string conflated the two. +preg_match('#
#s', $markup, $groupOnly); +$actionGroup = $groupOnly[0] ?? ''; $t->check( - 'the buttons are filled btn-secondary, not outline', - false !== strpos($markup, 'class="btn btn-secondary fog-quicktask"') - && false === strpos($markup, 'btn-outline-secondary') + 'the card buttons are filled btn-secondary, not outline', + false !== strpos($actionGroup, 'class="btn btn-secondary fog-quicktask"') + && false === strpos($actionGroup, 'btn-outline-secondary') +); + +// ------------------------------------------------------------------------- +// 2b. The confirmation modal. +// ------------------------------------------------------------------------- +// The buttons carry data-confirm, but that attribute only matters if +// something reads it. window.confirm() used to; now it is this modal, and a +// modal that failed to render would leave the script showing nothing and +// firing nothing -- a dead button, not an unguarded one, but still broken. +$t->check( + 'a confirmation modal ships with the buttons', + false !== strpos($markup, 'id="quicktask-confirm-modal"') +); +$t->check( + 'it has the body the script writes the confirmation into', + false !== strpos($markup, 'id="quicktask-confirm-text"') +); +$t->check( + 'it can be dismissed without tasking anything', + false !== strpos($markup, 'id="quicktask-confirm-cancel"') + && false !== strpos($markup, 'data-bs-dismiss="modal"') +); +$t->check( + 'and it has the commit button the script fires on', + false !== strpos($markup, 'id="quicktask-confirm-go"') +); +// No modal without buttons: a page whose task types are all gone emits +// neither, rather than a confirmation for nothing. +$t->check( + 'no buttons means no modal either', + false === strpos($emit(['*'], 'host', [9998, 9999]), 'quicktask-confirm-modal') ); $t->check( 'a task type this server has deleted simply loses its button',