Refactor the toolbar html & css to improve its overall accessibility …

…(bug 1171799, bug 1855695) The first goal of this patch was to remove the tabindex because it helps to improve overall a11y. That led to move some html elements associated with the buttons which helped to position these elements relatively to their buttons. Consequently it was easy to change the toolbar height (configurable in Firefox with the pref browser.uidensity): it's the second goal of this patch. For a11y reasons we want to be able to change the height of the toolbar to make the buttons larger.
mozilla · Sep 20, 2024 · 432c954 · 432c954
1 parent 6222359
commit 432c954
Show file tree

Hide file tree

Showing 8 changed files with 1,029 additions and 855 deletions.
diff --git a/test/integration/find_spec.mjs b/test/integration/find_spec.mjs
@@ -43,7 +43,7 @@ describe("find bar", () => {
           await page.click("#viewFindButton");
           await page.waitForSelector("#viewFindButton", { hidden: false });
           await page.type("#findInput", "a");
-          await page.click("#findHighlightAll");
+          await page.click("#findHighlightAll + label");
           await page.waitForSelector(".textLayer .highlight");
 
           // The PDF file contains the text 'AB BA' in a monospace font on a

diff --git a/web/annotation_editor_layer_builder.css b/web/annotation_editor_layer_builder.css
@@ -1227,18 +1227,9 @@
 }
 
 #highlightParamsToolbarContainer {
-  height: auto;
-  padding-inline: 10px;
-  padding-block: 10px 16px;
   gap: 16px;
-  display: flex;
-  flex-direction: column;
-  box-sizing: border-box;
-
-  .editorParamsLabel {
-    width: fit-content;
-    inset-inline-start: 0;
-  }
+  padding-inline: 10px;
+  padding-block-end: 12px;
 
   .colorPicker {
     display: flex;
@@ -1262,6 +1253,7 @@
         align-items: center;
         background: none;
         flex: 0 0 auto;
+        padding: 0;
 
         .swatch {
           width: 24px;
@@ -1291,7 +1283,6 @@
     align-self: stretch;
 
     .editorParamsLabel {
-      width: 100%;
       height: auto;
       align-self: stretch;
     }

diff --git a/web/app.js b/web/app.js
@@ -515,7 +515,11 @@ const PDFViewerApplication = {
     }
 
     if (!this.supportsIntegratedFind && appConfig.findBar) {
-      this.findBar = new PDFFindBar(appConfig.findBar, eventBus);
+      this.findBar = new PDFFindBar(
+        appConfig.findBar,
+        appConfig.mainContainer,
+        eventBus
+      );
     }
 
     if (appConfig.annotationEditorParams) {

diff --git a/web/pdf_find_bar.js b/web/pdf_find_bar.js
@@ -25,9 +25,11 @@ const MATCHES_COUNT_LIMIT = 1000;
  * is done by PDFFindController.
  */
 class PDFFindBar {
+  #mainContainer;
+
   #resizeObserver = new ResizeObserver(this.#resizeObserverCallback.bind(this));
 
-  constructor(options, eventBus) {
+  constructor(options, mainContainer, eventBus) {
     this.opened = false;
 
     this.bar = options.bar;
@@ -42,6 +44,7 @@ class PDFFindBar {
     this.findPreviousButton = options.findPreviousButton;
     this.findNextButton = options.findNextButton;
     this.eventBus = eventBus;
+    this.#mainContainer = mainContainer;
 
     // Add event listeners to the DOM elements.
     this.toggleButton.addEventListener("click", () => {
@@ -170,7 +173,7 @@ class PDFFindBar {
       //  - The width of the viewer itself changes.
       //  - The width of the findbar changes, by toggling the visibility
       //    (or localization) of find count/status messages.
-      this.#resizeObserver.observe(this.bar.parentNode);
+      this.#resizeObserver.observe(this.#mainContainer);
       this.#resizeObserver.observe(this.bar);
 
       this.opened = true;
@@ -184,6 +187,7 @@ class PDFFindBar {
     if (!this.opened) {
       return;
     }
+
     this.#resizeObserver.disconnect();
 
     this.opened = false;
@@ -200,7 +204,7 @@ class PDFFindBar {
     }
   }
 
-  #resizeObserverCallback(entries) {
+  #resizeObserverCallback(_entries) {
     const { bar } = this;
     // The find bar has an absolute position and thus the browser extends
     // its width to the maximum possible width once the find bar does not fit

diff --git a/web/toolbar.js b/web/toolbar.js
@@ -21,7 +21,7 @@ import {
   DEFAULT_SCALE_VALUE,
   MAX_SCALE,
   MIN_SCALE,
-  toggleCheckedBtn,
+  toggleExpandedBtn,
 } from "./ui_utils.js";
 
 /**
@@ -180,6 +180,9 @@ class Toolbar {
     for (const { element, eventName, eventDetails, telemetry } of buttons) {
       element.addEventListener("click", evt => {
         if (eventName !== null) {
+          if (evt.target !== element) {
+            return;
+          }
           eventBus.dispatch(eventName, {
             source: this,
             ...eventDetails,
@@ -270,32 +273,32 @@ class Toolbar {
       editorStampParamsToolbar,
     } = this.#opts;
 
-    toggleCheckedBtn(
+    toggleExpandedBtn(
       editorFreeTextButton,
       mode === AnnotationEditorType.FREETEXT,
       editorFreeTextParamsToolbar
     );
-    toggleCheckedBtn(
+    toggleExpandedBtn(
       editorHighlightButton,
       mode === AnnotationEditorType.HIGHLIGHT,
       editorHighlightParamsToolbar
     );
-    toggleCheckedBtn(
+    toggleExpandedBtn(
       editorInkButton,
       mode === AnnotationEditorType.INK,
       editorInkParamsToolbar
     );
-    toggleCheckedBtn(
+    toggleExpandedBtn(
       editorStampButton,
       mode === AnnotationEditorType.STAMP,
       editorStampParamsToolbar
     );
 
     const isDisable = mode === AnnotationEditorType.DISABLE;
-    editorFreeTextButton.disabled = isDisable;
-    editorHighlightButton.disabled = isDisable;
-    editorInkButton.disabled = isDisable;
-    editorStampButton.disabled = isDisable;
+    editorFreeTextButton.toggleAttribute("disabled", isDisable);
+    editorHighlightButton.toggleAttribute("disabled", isDisable);
+    editorInkButton.toggleAttribute("disabled", isDisable);
+    editorStampButton.toggleAttribute("disabled", isDisable);
   }
 
   #updateUIState(resetNumPages = false) {

diff --git a/web/viewer-geckoview.html b/web/viewer-geckoview.html
@@ -93,13 +93,13 @@
 
   </head>
 
-  <body tabindex="1">
+  <body tabindex="0">
     <div id="outerContainer">
 
       <div id="mainContainer">
 
         <div id="floatingToolbar">
-          <button id="download" class="toolbarButton" type="button" title="Download" tabindex="31" data-l10n-id="pdfjs-download-button">
+          <button id="download" class="toolbarButton" type="button" title="Download" tabindex="0" data-l10n-id="pdfjs-download-button">
             <span data-l10n-id="pdfjs-download-button-label">Download</span>
           </button>
         </div>