Merge pull request #18385 from calixteman/remove_tabindex

Refactor the toolbar html & css to improve its overall accessibility (bug 1171799, bug 1855695)
mozilla · Sep 24, 2024 · 313ab9a · 313ab9a
2 parents c90ce5d + 6f40ed7
commit 313ab9a
Show file tree

Hide file tree

Showing 9 changed files with 975 additions and 807 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
@@ -516,7 +516,11 @@ const PDFViewerApplication = {
     }
 
     if (!this.supportsIntegratedFind && appConfig.findBar) {
-      this.findBar = new PDFFindBar(appConfig.findBar, eventBus);
+      this.findBar = new PDFFindBar(
+        appConfig.findBar,
+        appConfig.principalContainer,
+        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;
@@ -200,7 +203,7 @@ class PDFFindBar {
     }
   }
 
-  #resizeObserverCallback(entries) {
+  #resizeObserverCallback() {
     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";
 
 /**
@@ -270,22 +270,22 @@ 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

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>