diff --git a/instances/templates/instances/settings_tab.html b/instances/templates/instances/settings_tab.html index 26c576fb..62daa717 100644 --- a/instances/templates/instances/settings_tab.html +++ b/instances/templates/instances/settings_tab.html @@ -246,7 +246,7 @@ - +
@@ -328,7 +328,7 @@ {% if network.type != "hostdev" %} {% endif %} @@ -467,8 +467,7 @@ + data-confirm="{% trans "Remove this USB device from the VM? A running VM loses it at once." %}">{% bs_icon 'trash' %} {% endif %} @@ -501,7 +500,7 @@ + data-confirm="{% trans "Remove this PCI device from the VM? A running VM's guest has to let it go." %}">{% bs_icon 'trash' %} {% endif %} diff --git a/instances/templates/instances/snapshots_tab.html b/instances/templates/instances/snapshots_tab.html index f9a22e1f..38096858 100755 --- a/instances/templates/instances/snapshots_tab.html +++ b/instances/templates/instances/snapshots_tab.html @@ -87,27 +87,27 @@ + {% if busy or held %} disabled{% endif %}>{% trans "Revert" %} {% else %} {% endif %} diff --git a/instances/tests/test_states.py b/instances/tests/test_states.py index 1a977895..2164fba8 100644 --- a/instances/tests/test_states.py +++ b/instances/tests/test_states.py @@ -98,7 +98,7 @@ class StateTests(TestCase): for action in {"force_off", "powercycle"} & ACTIONS[state]: button = page.xpath(f"//form[@action='{reverse(f'instances:{action}', args=[vm.id])}']//button")[0] with self.subTest(url=url, vm=vm.name, action=action): - self.assertEqual(button.get("onclick"), "return confirm(this.dataset.confirm)") + self.assertIsNone(button.get("onclick")) # the page's dialog asks, not the browser self.assertIn(vm.name, button.get("data-confirm")) self.assertIn("power cut", button.get("data-confirm")) diff --git a/interfaces/templates/interface.html b/interfaces/templates/interface.html index 3bf58ada..bc1e6c6f 100644 --- a/interfaces/templates/interface.html +++ b/interfaces/templates/interface.html @@ -13,8 +13,7 @@ diff --git a/nwfilters/templates/nwfilter.html b/nwfilters/templates/nwfilter.html index bd25340b..40c5315a 100644 --- a/nwfilters/templates/nwfilter.html +++ b/nwfilters/templates/nwfilter.html @@ -69,8 +69,7 @@ + data-confirm="{% blocktrans with ref=ref.filter %}Remove the reference to {{ ref }}? Its rules no longer apply through this filter.{% endblocktrans %}">{% bs_icon 'trash' %} @@ -125,8 +124,7 @@ + data-confirm="{% trans "Delete this rule? Interfaces that use the filter no longer get it." %}">{% bs_icon 'trash' %} diff --git a/nwfilters/templates/nwfilters.html b/nwfilters/templates/nwfilters.html index 2a7e0f5a..e6ecaacf 100644 --- a/nwfilters/templates/nwfilters.html +++ b/nwfilters/templates/nwfilters.html @@ -60,8 +60,7 @@ {# a filter in use is not deleted: a VM using it, or a filter referencing it, would not start #} + data-confirm="{% blocktrans with name=nwf.name %}Delete filter {{ name }}?{% endblocktrans %}">{% bs_icon 'trash' %} {% if nwf.vms or nwf.referenced_by %}{% trans "Delete (in use)" %}{% else %}{% trans "Delete filter" %}{% endif %} diff --git a/static/js/confirm_dialog.js b/static/js/confirm_dialog.js new file mode 100644 index 00000000..d24fbf02 --- /dev/null +++ b/static/js/confirm_dialog.js @@ -0,0 +1,46 @@ +// A button with data-confirm asks in the page's confirm dialog before it acts: +// the dialog's title and its confirming button carry the button's own words +// (or its accessible name, for an icon), its body the data-confirm question. +// Confirmed, the button is clicked again, so its form sends its name and value. +// A button in another dialog hides that one while asking (Bootstrap does not +// stack dialogs) and shows it again after; Cancel gives the focus back. +document.addEventListener("click", function (event) { + var button = event.target.closest("button[data-confirm]"); + if (!button || button.dataset.confirmed) return; + event.preventDefault(); + var dialog = document.getElementById("confirm-dialog"); + var parent = button.closest(".modal"); + var action = button.textContent.trim() || button.getAttribute("aria-label") || button.title; + var ok = dialog.querySelector("[data-confirm-ok]"); + var confirmed = false; + dialog.querySelector(".modal-title").textContent = action; + dialog.querySelector(".modal-body").textContent = button.dataset.confirm; + ok.textContent = action; + ok.className = "btn " + (/\b(btn-|btn-outline-|text-)danger\b/.test(button.className) ? "btn-danger" : "btn-primary"); + ok.disabled = false; + ok.onclick = function () { + ok.disabled = true; // once: the dialog is still open while it hides + confirmed = true; + bootstrap.Modal.getInstance(dialog).hide(); + button.dataset.confirmed = "1"; + button.click(); + delete button.dataset.confirmed; + }; + dialog.addEventListener("hidden.bs.modal", function () { + if (parent) { + // a form sent in place (the flavors) keeps its dialog; a page sent away does not care + bootstrap.Modal.getOrCreateInstance(parent).show(); + } else if (!confirmed) { + // a menu's item is hidden with its menu: its toggle takes the focus + var menu = button.closest(".dropdown"); + (menu ? menu.querySelector("[data-bs-toggle=dropdown]") : button).focus(); + } + }, {once: true}); + var ask = function () { bootstrap.Modal.getOrCreateInstance(dialog).show(); }; + if (parent) { + parent.addEventListener("hidden.bs.modal", ask, {once: true}); + bootstrap.Modal.getInstance(parent).hide(); + } else { + ask(); + } +}, true); diff --git a/storages/templates/create_stg_vol_block.html b/storages/templates/create_stg_vol_block.html index af2b1295..9e2790ca 100644 --- a/storages/templates/create_stg_vol_block.html +++ b/storages/templates/create_stg_vol_block.html @@ -24,7 +24,7 @@ {% trans "Starting upload..." %} - + @@ -257,20 +253,20 @@ if (uploadInProgress) return; var fileInput = $('#id_file')[0]; + var errorMessage = $('#upload-error-message'); if (fileInput.files.length === 0) { - alert("{% trans 'Please select a file to upload.'|escapejs %}"); + errorMessage.text("{% trans 'Please select a file to upload.'|escapejs %}").removeClass("d-none"); return; } var file = fileInput.files[0]; var fileName = file.name; - var errorMessage = $('#upload-error-message'); if (!/\.iso$/i.test(fileName) || fileName.length <= 4) { errorMessage.text("{% trans 'Only .iso files can be uploaded.'|escapejs %}").removeClass("d-none"); return; } if (file.size === 0) { - alert("{% trans 'The selected file is empty.'|escapejs %}"); + errorMessage.text("{% trans 'The selected file is empty.'|escapejs %}").removeClass("d-none"); return; } var chunkSize = 32 * 1024 * 1024; // Fewer SSH connections per upload diff --git a/templates/base.html b/templates/base.html index 446f6156..a8d040f3 100644 --- a/templates/base.html +++ b/templates/base.html @@ -59,6 +59,23 @@ {% block content %}{% endblock %} + {# the dialog every button with data-confirm asks in (confirm_dialog.js) #} + + @@ -66,6 +83,7 @@ + diff --git a/virtsecrets/templates/secrets.html b/virtsecrets/templates/secrets.html index a06f566f..87c044de 100644 --- a/virtsecrets/templates/secrets.html +++ b/virtsecrets/templates/secrets.html @@ -72,8 +72,7 @@ {# a secret in use is not deleted: the VM or pool using it would not start #} + data-confirm="{% blocktrans with usage=secret.usage|default:secret.uuid %}Delete secret {{ usage }}? Its stored value will be lost. No current or saved definition of a VM or pool names it; snapshots, LUKS volumes and qemu.conf's TLS settings are not checked.{% endblocktrans %}">{% bs_icon 'trash' %} {% if secret.vms or secret.pools %}{% trans "Delete (in use)" %}{% else %}{% trans "Delete secret" %}{% endif %} diff --git a/webvirtcloud/tests/test_confirm_prompts.py b/webvirtcloud/tests/test_confirm_prompts.py index 7595488a..16fef2f3 100644 --- a/webvirtcloud/tests/test_confirm_prompts.py +++ b/webvirtcloud/tests/test_confirm_prompts.py @@ -1,29 +1,32 @@ -"""A confirmation says what its action does: every prompt in a template is the -button's own data-confirm text, shown by return confirm(this.dataset.confirm).""" +"""A confirmation says what its action does, in the page's dialog: a button +asks its own data-confirm question through confirm_dialog.js, never the +browser's confirm().""" +import re from pathlib import Path from django.conf import settings from django.test import SimpleTestCase -ONCLICK = 'onclick="return confirm(this.dataset.confirm)"' - class ConfirmPromptTests(SimpleTestCase): - def test_every_prompt_is_the_buttons_own_text(self): + def test_every_prompt_is_a_buttons_own_text_in_the_pages_dialog(self): base = Path(settings.BASE_DIR) templates = [*base.glob("*/templates/**/*.html"), *base.glob("templates/**/*.html")] self.assertTrue(templates) for path in templates: text = path.read_text() - at = text.find("confirm(") - while at != -1: + with self.subTest(template=str(path.relative_to(base))): + self.assertNotIn("confirm(", text) + for match in re.finditer(r"data-confirm=", text): # the tag around it: prompt texts and template tags hold no < or > - tag = text[text.rfind("<", 0, at):text.find(">", at) + 1] + tag = text[text.rfind("<", 0, match.start()):text.find(">", match.start()) + 1] + end = text.find("", match.start()) + content = re.sub(r"\{%\s*(bs_icon|if|elif|else|endif)\b[^%]*%\}", "", + text[text.find(">", match.start()) + 1:end]).strip() with self.subTest(template=str(path.relative_to(base)), tag=tag[:60]): - self.assertRegex(tag, r"^<(button|input|a)\b") - self.assertIn(ONCLICK, tag) - self.assertIn('data-confirm="', tag) + self.assertRegex(tag, r"^