security/acme-client: Fix certificate grid button handlers not working

[NormB]

Summary

Fixes certificate grid action buttons (Issue/Renew, Re-Import, Run Automations, Revoke, Reset) not working correctly with the new UIBootgrid/Tabulator system in OPNsense 25.7.x.

Problems Fixed

1. Buttons not firing (no popup appearing)

• Clicks on action buttons were being intercepted by Tabulator's row selection
• Fix: Added e.stopPropagation() and e.preventDefault() to all command handlers

2. Multiple clicks required to close dialogs

• The loaded.rs.jquery.bootgrid event fires on every grid reload
• Handlers attached inside this callback would stack up (N reloads = N handlers)
• Each click would open multiple dialogs stacked on top of each other
• Fix: Moved custom command handlers outside the callback and used namespaced events with .off() before .on() to ensure single handler

Changes

• Custom command handlers (toggle, sign, revoke, removekey, automation, import) now bound at document level with namespaced events (click.acme.toggle, etc.)
• Added e.stopPropagation() and e.preventDefault() to prevent Tabulator interference
• Used .off('click').on() pattern for handlers that must remain inside the callback

Testing

• Verified all action buttons show confirmation dialogs with single click
• Verified dialogs close with single click on Yes/Cancel
• Verified actions execute correctly after confirmation

Fixes #5123

Co-Authored-By: Claude Opus 4.5 noreply@anthropic.com

[fraenki]

That's a HUGE diff and as a result hard to review. I'd prefer if you could modify your PR to only include the minimum changes required to fix your issue. I'd assume that the change to off('click').on('click', function() is the only relevant change, but I can't test it (because I can't reproduce the issue that you are trying to solve).

Could you please update your PR and remove all changes that are not strictly necessary? Or otherwise please elaborate why this huge change is necessary and explain the reson for each individual change.

[Famaku]

Can confirm the issue on OPNsense 26.1.1-amd64.
Buttons to handle issue/renew, revoke, import, run automations, revole, reset for any certificate do not work. Instead of triggering the desired action, the entry row is simply highlighted.

[fraenki]

@Famaku This PR is the wrong place to discuss your issue.

[NormB]

Suggestion: Simplified approach with fewer line changes

This PR can be significantly simplified while achieving the same fix. Instead of moving handlers outside the callback and restructuring the code, we can make minimal in-place changes:

Current PR stats

• 138 additions / 135 deletions (~275 lines changed)

Simplified approach stats

• ~31 lines changed

The simplified approach

Keep all handlers in their original locations and just add two fixes:

1. Add .off("click") before .on("click") - prevents handler stacking from multiple grid reloads
2. Add e.stopPropagation(); e.preventDefault(); - prevents Tabulator row selection from intercepting clicks

Example transformation:

// Before
grid_certificates.find(".command-toggle").on("click", function(e)
{
    if (gridParams['toggle'] != undefined) {

// After  
grid_certificates.find(".command-toggle").off("click").on("click", function(e)
{
    e.stopPropagation();
    e.preventDefault();
    if (gridParams['toggle'] != undefined) {

Full simplified diff

diff --git a/security/acme-client/src/opnsense/mvc/app/views/OPNsense/AcmeClient/certificates.volt b/security/acme-client/src/opnsense/mvc/app/views/OPNsense/AcmeClient/certificates.volt
--- a/security/acme-client/src/opnsense/mvc/app/views/OPNsense/AcmeClient/certificates.volt
+++ b/security/acme-client/src/opnsense/mvc/app/views/OPNsense/AcmeClient/certificates.volt
@@ -141,7 +141,7 @@ POSSIBILITY OF SUCH DAMAGE.
          */
         const grid_certificates = $("#grid-certificates").UIBootgrid($.extend(gridParams, { options: gridopt }));
 
-        $("#grid_certificates").on("loaded.rs.jquery.bootgrid", function (e)
+        grid_certificates.on("loaded.rs.jquery.bootgrid", function (e)
         {
             // toggle all rendered tooltips (once for all)
             $('.bootgrid-tooltip').tooltip();
@@ -157,7 +157,7 @@ POSSIBILITY OF SUCH DAMAGE.
             var gridId = $(this).attr('id');
 
             // link Add new to child button with data-action = add
-            $(this).find("*[data-action=add]").click(function(){
+            $(this).find("*[data-action=add]").off('click').on('click', function(){
                 if ( gridParams['get'] != undefined && gridParams['add'] != undefined) {
                     var urlMap = {};
                     urlMap['frm_' + editDlg] = gridParams['get'];
@@ -185,7 +185,7 @@ POSSIBILITY OF SUCH DAMAGE.
             });
 
             // link delete selected items action
-            $(this).find("*[data-action=deleteSelected]").click(function(){
+            $(this).find("*[data-action=deleteSelected]").off('click').on('click', function(){
                 if ( gridParams['del'] != undefined) {
                     stdDialogConfirm('{{ lang._('Confirm removal') }}',
                         '{{ lang._('Do you want to remove the selected item?') }}',
@@ -214,8 +214,10 @@ POSSIBILITY OF SUCH DAMAGE.
             var gridId = $(this).attr('id');
 
             // edit item
-            grid_certificates.find(".command-edit").on("click", function(e)
+            grid_certificates.find(".command-edit").off("click").on("click", function(e)
             {
+                e.stopPropagation();
+                e.preventDefault();
                 if (editDlg != undefined && gridParams['get'] != undefined) {
                     var uuid = $(this).data("row-id");
                     var urlMap = {};
@@ -247,8 +249,10 @@ POSSIBILITY OF SUCH DAMAGE.
             });
 
             // copy item, save as new
-            grid_certificates.find(".command-copy").on("click", function(e)
+            grid_certificates.find(".command-copy").off("click").on("click", function(e)
             {
+                e.stopPropagation();
+                e.preventDefault();
                 if (editDlg != undefined && gridParams['get'] != undefined) {
                     var uuid = $(this).data("row-id");
                     var urlMap = {};
@@ -280,8 +284,10 @@ POSSIBILITY OF SUCH DAMAGE.
             });
 
             // delete item
-            grid_certificates.find(".command-delete").on("click", function(e)
+            grid_certificates.find(".command-delete").off("click").on("click", function(e)
             {
+                e.stopPropagation();
+                e.preventDefault();
                 if (gridParams['del'] != undefined) {
                     var uuid=$(this).data("row-id");
                     stdDialogConfirm('{{ lang._('Confirm removal') }}',
@@ -298,8 +304,10 @@ POSSIBILITY OF SUCH DAMAGE.
             });
 
             // toggle item
-            grid_certificates.find(".command-toggle").on("click", function(e)
+            grid_certificates.find(".command-toggle").off("click").on("click", function(e)
             {
+                e.stopPropagation();
+                e.preventDefault();
                 if (gridParams['toggle'] != undefined) {
                     var uuid=$(this).data("row-id");
                     $(this).addClass("fa-spinner fa-pulse");
@@ -314,8 +322,10 @@ POSSIBILITY OF SUCH DAMAGE.
             });
 
             // sign cert
-            grid_certificates.find(".command-sign").on("click", function(e)
+            grid_certificates.find(".command-sign").off("click").on("click", function(e)
             {
+                e.stopPropagation();
+                e.preventDefault();
                 if (gridParams['sign'] != undefined) {
                     var uuid=$(this).data("row-id");
                     stdDialogConfirm('{{ lang._('Confirmation Required') }}',
@@ -335,8 +345,10 @@ POSSIBILITY OF SUCH DAMAGE.
             });
 
             // revoke cert
-            grid_certificates.find(".command-revoke").on("click", function(e)
+            grid_certificates.find(".command-revoke").off("click").on("click", function(e)
             {
+                e.stopPropagation();
+                e.preventDefault();
                 if (gridParams['revoke'] != undefined) {
                     var uuid=$(this).data("row-id");
                     stdDialogConfirm('{{ lang._('Confirmation Required') }}',
@@ -354,8 +366,10 @@ POSSIBILITY OF SUCH DAMAGE.
             });
 
             // remove private key
-            grid_certificates.find(".command-removekey").on("click", function(e)
+            grid_certificates.find(".command-removekey").off("click").on("click", function(e)
             {
+                e.stopPropagation();
+                e.preventDefault();
                 if (gridParams['removekey'] != undefined) {
                     var uuid=$(this).data("row-id");
                     stdDialogConfirm('{{ lang._('Confirmation Required') }}',
@@ -373,8 +387,10 @@ POSSIBILITY OF SUCH DAMAGE.
             });
 
             // run automation
-            grid_certificates.find(".command-automation").on("click", function(e)
+            grid_certificates.find(".command-automation").off("click").on("click", function(e)
             {
+                e.stopPropagation();
+                e.preventDefault();
                 if (gridParams['automation'] != undefined) {
                     var uuid=$(this).data("row-id");
                     stdDialogConfirm('{{ lang._('Confirmation Required') }}',
@@ -392,8 +408,10 @@ POSSIBILITY OF SUCH DAMAGE.
             });
 
             // import certificate into trust storage
-            grid_certificates.find(".command-import").on("click", function(e)
+            grid_certificates.find(".command-import").off("click").on("click", function(e)
             {
+                e.stopPropagation();
+                e.preventDefault();
                 if (gridParams['import'] != undefined) {
                     var uuid=$(this).data("row-id");
                     stdDialogConfirm('{{ lang._('Confirmation Required') }}',

Summary of changes

Change	Count
Fix selector: $("#grid_certificates") → grid_certificates	1
Add .off('click') before .on('click')	11 handlers
Add e.stopPropagation(); e.preventDefault();	9 handlers

Benefits of this approach

• Minimal diff - easier to review
• Keeps code structure unchanged - less risk of introducing regressions
• Fixes the same issues - handler stacking + Tabulator click interference
• Also fixes a latent bug - the original code had $("#grid_certificates") (underscore) which didn't match the element ID grid-certificates (hyphen)

Trade-off

The current PR's document-level delegation approach is slightly more efficient since handlers are bound once globally rather than rebound on every grid reload. However, this performance difference is negligible for a UI like this.

[Monviech]

I want to talk to a person please :(