Accessibility: name the Quill editor controls, drop two eval calls

Follow-up to the accessibility pass, from testing the PR preview and the
SonarQube report.

SonarQube quality gate (D security rating on new code)
- bx_menu_popup() no longer evaluates string callbacks. The two eval calls it
  reported were introduced by this branch: wrapping onShow/onHide to toggle the
  trigger's aria-expanded meant replicating dolPopup's own handling of string
  callbacks. The wrapper is now installed only when the callback is not a
  string, so a string onShow is handed back to dolPopup, which evaluates it in
  code this branch does not touch. No call site passes either callback, so
  behaviour is unchanged for all nineteen of them.

Quill editor, which a default install selects and the preview therefore uses
- The mention plugin points aria-owns at a list it only inserts into the
  document once the list opens, so every page carrying an editor reported an
  invalid ARIA value. The editor is not a combobox, so the attribute is
  dropped; mentions still open and close as before.
- The four toolbar pickers carry role="button" on an inner element, so the
  title sat on a wrapper that was never the control. Alignment also renders as
  four plain buttons in the smaller toolbars, and embed, emoji and edit-HTML
  had no name at all. Six language strings are added for the labels that did
  not exist yet.

Disclosure state
- The toolbar notifications bell no longer claims aria-expanded. Its popup is
  anchored on the ancestor list rather than on the link, so nothing was in a
  position to keep the value honest and it read collapsed while open.
- bx_menu_popup() now leaves aria-expanded alone on a trigger that names its
  own target with aria-controls. Such a trigger belongs to whatever it names,
  a sidebar or the search box, and that owner keeps its state; writing to it
  from here left the account sidebar trigger describing the wrong thing.

Verified with the editor active: all fourteen member pages and the audited
guest pages score 100.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

inc/js/editor.quill.js

@@ -423,12 +423,23 @@ function bx_editor_init(oParams) {
     $(oParams.selector).next().find('button.ql-link').attr('title', _t('_sys_txt_quill_tooltip_link'));
     $(oParams.selector).next().find('button.ql-image').attr('title', _t('_sys_txt_quill_tooltip_image'));
     $(oParams.selector).next().find('button.ql-code-block').attr('title', _t('_sys_txt_quill_tooltip_code_block'));
-    $(oParams.selector).next().find('span.ql-color').attr('title', _t('_sys_txt_quill_tooltip_color'));
-    $(oParams.selector).next().find('span.ql-background').attr('title', _t('_sys_txt_quill_tooltip_background'));
-    $(oParams.selector).next().find('span.ql-align').attr('title', _t('_sys_txt_quill_tooltip_align'));
-    $(oParams.selector).next().find('span.ql-header').attr('title', _t('_sys_txt_quill_tooltip_header'));
+    $(oParams.selector).next().find('button.ql-embed').attr('title', _t('_sys_txt_quill_tooltip_embed'));
+    $(oParams.selector).next().find('button.ql-emoji').attr('title', _t('_sys_txt_quill_tooltip_emoji'));
+    $(oParams.selector).next().find('button.ql-show-html').attr('title', _t('_sys_txt_quill_tooltip_show_html'));
+    $(oParams.selector).next().find('span.ql-color .ql-picker-label').attr({'title': _t('_sys_txt_quill_tooltip_color'), 'aria-label': _t('_sys_txt_quill_tooltip_color')});
+    $(oParams.selector).next().find('span.ql-background .ql-picker-label').attr({'title': _t('_sys_txt_quill_tooltip_background'), 'aria-label': _t('_sys_txt_quill_tooltip_background')});
+    $(oParams.selector).next().find('span.ql-align .ql-picker-label').attr({'title': _t('_sys_txt_quill_tooltip_align'), 'aria-label': _t('_sys_txt_quill_tooltip_align')});
+    //--- a mini toolbar renders alignment as four separate buttons rather than one picker
+    $(oParams.selector).next().find('button.ql-align[value=""]').attr('title', _t('_sys_txt_quill_tooltip_align_left'));
+    $(oParams.selector).next().find('button.ql-align[value="center"]').attr('title', _t('_sys_txt_quill_tooltip_align_center'));
+    $(oParams.selector).next().find('button.ql-align[value="right"]').attr('title', _t('_sys_txt_quill_tooltip_align_right'));
+    $(oParams.selector).next().find('button.ql-align[value="justify"]').attr('title', _t('_sys_txt_quill_tooltip_align_justify'));
+    $(oParams.selector).next().find('span.ql-header .ql-picker-label').attr({'title': _t('_sys_txt_quill_tooltip_header'), 'aria-label': _t('_sys_txt_quill_tooltip_header')});
 
-    $('#' + oParams.name).find('.ql-editor').addClass('bx-def-vanilla-html');
+    //--- quill-mention points aria-owns at its list, but only inserts that list into the document when
+    //--- the list opens, so the reference dangles and every page with an editor reports an invalid
+    //--- ARIA value. The editor is not a combobox, so the attribute has nothing to say here.
+    $('#' + oParams.name).find('.ql-editor').addClass('bx-def-vanilla-html').removeAttr('aria-owns');
 
     if (bEmptyToolbar)
         $(oParams.selector).next().hide();

inc/js/functions.js

@@ -501,15 +501,17 @@ function bx_menu_popup (o, e, options, vars) {
     if ('undefined' == typeof(e))
         e = window;
 
-    var bExpanded = e !== window && $(e).length != 0 && $(e).is('[aria-expanded]');
+    //--- Only a trigger that names no target of its own is this popup's disclosure. A trigger carrying
+    //--- aria-controls belongs to whatever it names - a sidebar, the search box - and that owner keeps
+    //--- its state; writing to it from here would leave it describing the wrong thing.
+    var bExpanded = e !== window && $(e).length != 0 && $(e).is('[aria-expanded]') && !$(e).is('[aria-controls]');
     var fOnShow = options.onShow, fOnHide = options.onHide;
 
-    var o = $.extend({}, $.fn.dolPopupDefaultOptions, {
-        id: o, 
-        url: bx_append_url_params('menu.php', $.extend({o:o}, vars)), 
-        cssClass: 'bx-popup-menu'
-    }, options, {
-        onShow: function(oPopup) {
+    // a string onShow is handed back to dolPopup, which evaluates it; a string onHide has never been
+    // supported on this path, since dolPopupAjax replaces onHide before dolPopup sees it
+    var aExpanded = {};
+    if(typeof fOnShow != 'string')
+        aExpanded.onShow = function(oPopup) {
             if(bExpanded)
                 $(e).attr('aria-expanded', 'true');
 
@@ -518,20 +520,23 @@ function bx_menu_popup (o, e, options, vars) {
 
             if(typeof fOnShow == 'function')
                 fOnShow(oPopup);
-            else if(typeof fOnShow == 'string')
-                eval(fOnShow);
-        },
-        onHide: function(oPopup) {
+        };
+
+    if(typeof fOnHide != 'string')
+        aExpanded.onHide = function(oPopup) {
             var oTrigger = oPopup.data('bx-popup-trigger');
-            if(oTrigger && oTrigger.length)
+            if(oTrigger)
                 oTrigger.attr('aria-expanded', 'false');
 
             if(typeof fOnHide == 'function')
                 fOnHide(oPopup);
-            else if(typeof fOnHide == 'string')
-                eval(fOnHide);
-        }
-    });
+        };
+
+    var o = $.extend({}, $.fn.dolPopupDefaultOptions, {
+        id: o,
+        url: bx_append_url_params('menu.php', $.extend({o:o}, vars)),
+        cssClass: 'bx-popup-menu'
+    }, options, aExpanded);
 
     $(e).dolPopupAjax(o);
 }

modules/boonex/artificer/data/template/system/_header_js_custom.html

@@ -89,7 +89,7 @@
         }
         else {
             var oReturnTo = bx_clicked_stack_pop();
-            $(oReturnTo ? oReturnTo : oTrigger.filter(':visible').get(0)).focus();
+            $(oReturnTo || oTrigger.filter(':visible').get(0)).focus();
         }
     }
 

modules/boonex/artificer/data/template/system/scripts/BxTemplMenuToolbar.php

@@ -80,8 +80,10 @@ class BxTemplMenuToolbar extends BxBaseMenuToolbar
                 if(isset($a['bx_if:onclick']['content']['onclick']))
                     $a['bx_if:onclick']['content']['onclick'] = str_replace(["bx_menu_slide", "'site', "], ['bx_menu_popup'], $a['bx_if:onclick']['content']['onclick']);
 
-                //--- No aria-controls: bx_menu_popup() creates the popup lazily, so the id does not exist yet.
-                $sAttrsAdd = 'aria-haspopup="dialog" aria-expanded="false"';
+                //--- No aria-controls, because bx_menu_popup() creates the popup lazily and the id does not
+                //--- exist yet, and no aria-expanded, because the popup is anchored on the ancestor <ul>
+                //--- rather than on this link, so nothing is in a position to keep the state honest.
+                $sAttrsAdd = 'aria-haspopup="dialog"';
                 break;
         }
 

modules/boonex/english/data/langs/system/en.xml

@@ -4536,8 +4536,14 @@
     <string name="_sys_txt_quill_tooltip_color"><![CDATA[Select a text color]]></string>
     <string name="_sys_txt_quill_tooltip_background"><![CDATA[Select a background color]]></string>
     <string name="_sys_txt_quill_tooltip_align"><![CDATA[Text alignment]]></string>
+    <string name="_sys_txt_quill_tooltip_align_center"><![CDATA[Align center]]></string>
+    <string name="_sys_txt_quill_tooltip_align_justify"><![CDATA[Justify]]></string>
+    <string name="_sys_txt_quill_tooltip_align_left"><![CDATA[Align left]]></string>
+    <string name="_sys_txt_quill_tooltip_align_right"><![CDATA[Align right]]></string>
     <string name="_sys_txt_quill_tooltip_header"><![CDATA[Select the text style]]></string>
     <string name="_sys_txt_quill_tooltip_embed"><![CDATA[Insert an embed]]></string>
+    <string name="_sys_txt_quill_tooltip_emoji"><![CDATA[Insert an emoji]]></string>
+    <string name="_sys_txt_quill_tooltip_show_html"><![CDATA[Edit HTML source]]></string>
     <string name="_sys_txt_quill_tooltip_embed_popup_header"><![CDATA[Paste an URL for embed]]></string>
     <string name="_sys_txt_quill_tooltip_link_popup_header"><![CDATA[Paste an URL]]></string>
     <string name="_sys_queue_emails"><![CDATA[Emails]]></string>