Repository: ymrl/a11y-visualizer
Pull request #139 (2024-06-18 14:50:41)

:sparkles: fix table header logic to fit to spec


Patch:
diff --git a/src/content/dom/tips/internalTable.ts b/src/content/dom/tips/internalTable.ts
index be58bcc..ebfa2d2 100644
--- a/src/content/dom/tips/internalTable.ts
+++ b/src/content/dom/tips/internalTable.ts
@@ -3,12 +3,13 @@ import { getKnownRole } from "../getKnownRole";
 type RowGroup = {
   positionY: number;
   sizeY: number;
-  element: Element | null;
+  element: Element;
 };
 
 type ColGroup = {
   positionX: number;
   sizeX: number;
+  element: Element;
 };
 
 type Scope = "row" | "col" | "rowgroup" | "colgroup" | "auto" | "none";
@@ -33,33 +34,6 @@ export class InternalTable {
     this.element = table;
     const tagName = table.tagName.toLowerCase();
     const rowElements = getRowElements(table);
-    const rowGroupElements = getRowGroupElements(table);
-
-    const { rowGroups } = rowElements.reduce(
-      (prev, row, rowIndex) => {
-        const { rowGroups, groupElements } = prev;
-        const prevGroup =
-          rowGroups.length > 0 ? rowGroups[rowGroups.length - 1] : null;
-        if (
-          prevGroup &&
-          ((prevGroup.element && prevGroup.element.contains(row)) ||
-            (!prevGroup.element && !rowGroupElements[0]) ||
-            (!prevGroup.element && !groupElements[0].contains(row)))
-        ) {
-          prevGroup.sizeY += 1;
-          return prev;
-        }
-        const element = groupElements[0]?.contains(row)
-          ? groupElements[0]
-          : null;
-        const restElements = element ? groupElements.slice(1) : groupElements;
-        return {
-          rowGroups: [...rowGroups, { positionY: rowIndex, sizeY: 1, element }],
-          groupElements: restElements,
-        };
-      },
-      { rowGroups: [] as RowGroup[], groupElements: rowGroupElements },
-    );
     const cells: Cell[][] = rowElements.reduce((rows, row, rowIndex) => {
       const cellElements = getCellElements(row);
       const prevRow = rows.length > 0 ? rows[rows.length - 1] : null;
@@ -112,14 +86,20 @@ export class InternalTable {
             }
           }
           const isNativeTag = ["th", "td"].includes(cellTagName);
-          const colSpan =
-            (!isNativeTag && cell.getAttribute("aria-colspan")) ||
-            cell.getAttribute("colspan");
-          const rowSpan =
-            (!isNativeTag && cell.getAttribute("aria-rowspan")) ||
-            cell.getAttribute("rowspan");
-          const sizeX = colSpan ? parseInt(colSpan, 10) : 1;
-          const sizeY = rowSpan ? parseInt(rowSpan, 10) : 1;
+          const ariaColSpan = !isNativeTag && cell.getAttribute("aria-colspan");
+          const ariaRowSpan = !isNativeTag && cell.getAttribute("aria-rowspan");
+          const nativeColSpan = isNativeTag && cell.getAttribute("colspan");
+          const nativeRowSpan = isNativeTag && cell.getAttribute("rowspan");
+          const sizeX = ariaColSpan
+            ? parseInt(ariaColSpan, 10)
+            : nativeColSpan
+              ? Math.min(parseInt(nativeColSpan, 10), 1000)
+              : 1;
+          const sizeY = ariaRowSpan
+            ? parseInt(ariaRowSpan, 10)
+            : nativeRowSpan
+              ? Math.min(parseInt(nativeRowSpan, 10), 65534)
+              : 1;
           const scopeAttr = cell.getAttribute("scope")?.toLowerCase();
           const headerScope: Scope =
             cellTagName === "th"
@@ -164,6 +144,36 @@ export class InternalTable {
             ),
           0,
         );
+
+    const rowGroupElements = getRowGroupElements(table);
+    const { rowGroups } = rowElements.reduce(
+      (prev, row, rowIndex) => {
+        const { rowGroups, groupElements } = prev;
+        const prevGroup =
+          rowGroups.length > 0 ? rowGroups[rowGroups.length - 1] : null;
+        if (
+          prevGroup &&
+          ((prevGroup.element && prevGroup.element.contains(row)) ||
+            (!prevGroup.element && !rowGroupElements[0]) ||
+            (!prevGroup.element && !groupElements[0].contains(row)))
+        ) {
+          prevGroup.sizeY += 1;
+          return prev;
+        }
+        if (groupElements[0]?.contains(row)) {
+          return {
+            rowGroups: [
+              ...rowGroups,
+              { positionY: rowIndex, sizeY: 1, element: groupElements[0] },
+            ],
+            groupElements: groupElements.slice(1),
+          };
+        }
+        return prev;
+      },
+      { rowGroups: [] as RowGroup[], groupElements: rowGroupElements },
+    );
+
     const colGroupElements =
       tagName === "table"
         ? [...table.children].filter(
@@ -176,7 +186,7 @@ export class InternalTable {
     );
     const colGroups: ColGroup[] =
       colGroupElements.length > 0
-        ? colGroupElements.reduce((groups, group, groupIndex) => {
+        ? colGroupElements.reduce((groups, group) => {
             const lastGroup =
               groups.length > 0 ? groups[groups.length - 1] : null;
             const positionX = lastGroup
@@ -184,27 +194,19 @@ export class InternalTable {
               : minPositionX;
             const span = group.getAttribute("span");
             const sizeX = span
-              ? parseInt(span, 10)
-              : [...group.querySelectorAll("col")].reduce((prev, col) => {
-                  const span = col.getAttribute("span");
-                  return prev + (span ? parseInt(span, 10) : 1);
-                }, 0);
-            if (
-              groupIndex === colGroupElements.length - 1 &&
-              positionX + sizeX < colCount
-            ) {
-              return [
-                ...groups,
-                { positionX, sizeX },
-                {
-                  positionX: positionX + sizeX,
-                  sizeX: colCount - (positionX + sizeX),
-                },
-              ];
-            }
-            return [...groups, { positionX, sizeX }];
+              ? Math.min(parseInt(span, 10), 1000)
+              : Math.max(
+                  [...group.querySelectorAll("col")].reduce((prev, col) => {
+                    const span = col.getAttribute("span");
+                    return (
+                      prev + (span ? Math.min(parseInt(span, 10), 1000) : 1)
+                    );
+                  }, 0),
+                  1,
+                );
+            return [...groups, { positionX, sizeX, element: group }];
           }, [] as ColGroup[])
-        : [{ positionX: 0, sizeX: colCount }];
+        : [];
     this.rowGroups = rowGroups;
     this.colGroups = colGroups;
     this.cells = cells;
@@ -271,6 +273,7 @@ export class InternalTable {
     return (
       headerScope === "row" ||
       (headerScope === "auto" &&
+        !this.isColHeader(cell) &&
         !this.cells.some((row) =>
           row.some(
             (c) =>
@@ -283,7 +286,7 @@ export class InternalTable {
   };
 
   getRowHeaderElements = (cell: Cell): Element[] => {
-    const { positionY, sizeY, positionX } = cell;
+    const { positionY, sizeY, positionX, element } = cell;
     const headers: Element[] = [];
     Array(sizeY)
       .fill(0)
@@ -319,11 +322,11 @@ export class InternalTable {
             });
           });
       });
-    return headers;
+    return headers.filter((el) => el !== element && !isEmptyCellElement(el));
   };
 
   getRowGroupHeaderElements = (cell: Cell): Element[] => {
-    const { positionY } = cell;
+    const { positionY, element } = cell;
     const rowGroup = this.rowGroups.find(
       (group) =>
         group.positionY <= positionY &&
@@ -340,11 +343,12 @@ export class InternalTable {
         ),
       )
       .flat()
-      .map((c) => c.element);
+      .map((c) => c.element)
+      .filter((el) => el !== element && !isEmptyCellElement(el));
   };
 
   getColHeaderElements = (cell: Cell): Element[] => {
-    const { positionY, positionX, sizeX } = cell;
+    const { positionY, positionX, sizeX, element } = cell;
     const headers: Element[] = [];
 
     Array(sizeX)
@@ -381,11 +385,11 @@ export class InternalTable {
             });
           });
       });
-    return headers;
+    return headers.filter((el) => el !== element && !isEmptyCellElement(el));
   };
 
   getColGroupHeaderElements = (cell: Cell): Element[] => {
-    const { positionY, positionX } = cell;
+    const { positionY, positionX, element } = cell;
     const colGroup = this.colGroups.find(
       (group) =>
         group.positionX <= positionX &&
@@ -403,7 +407,8 @@ export class InternalTable {
         ),
       )
       .flat()
-      .map((c) => c.element);
+      .map((c) => c.element)
+      .filter((el) => el !== element && !isEmptyCellElement(el));
   };
 
   getAttributeHeaderElements = (cell: Cell): Element[] => {
@@ -415,7 +420,10 @@ export class InternalTable {
     const headerIds = headers.split(" ");
     return headerIds
       .map((id) => this.element.querySelector(`th#${id}, td#${id}`))
-      .filter((el): el is Element => !!el);
+      .filter(
+        (el): el is Element =>
+          !!el && el !== element && !isEmptyCellElement(el),
+      );
   };
 }
 
@@ -428,9 +436,20 @@ export const getRowElements = (el: Element): Element[] => {
     role === "grid" ||
     role === "treegrid"
   ) {
-    return [...el.children].reduce((prev, child) => {
-      return [...prev, ...getRowElementsInElement(child, tagName === "table")];
-    }, [] as Element[]);
+    return [...el.children]
+      .sort((a, b) =>
+        b.tagName.toLowerCase() === "tfoot"
+          ? a.tagName.toLowerCase() !== "tfoot"
+            ? -1
+            : 1
+          : 1,
+      )
+      .reduce((prev, child) => {
+        return [
+          ...prev,
+          ...getRowElementsInElement(child, tagName === "table"),
+        ];
+      }, [] as Element[]);
   }
 
   return [];
@@ -446,9 +465,7 @@ const getRowElementsInElement = (
     return [el];
   } else if (
     (inTableElement && ["thead", "tbody", "tfoot"].includes(tagName)) ||
-    role === "rowgroup" ||
-    role === "presentation" ||
-    role === "none"
+    ["rowgroup", "none", "presentation"].includes(role as string)
   ) {
     return [...el.children].reduce((prev, child) => {
       return [...prev, ...getRowElementsInElement(child, inTableElement)];
@@ -466,12 +483,20 @@ export const getRowGroupElements = (el: Element): Element[] => {
     role === "grid" ||
     role === "treegrid"
   ) {
-    return [...el.children].reduce((prev, child) => {
-      return [
-        ...prev,
-        ...getRowGroupElementsInElement(child, tagName === "table"),
-      ];
-    }, [] as Element[]);
+    return [...el.children]
+      .sort((a, b) =>
+        b.tagName.toLowerCase() === "tfoot"
+          ? a.tagName.toLowerCase() !== "tfoot"
+            ? -1
+            : 1
+          : 1,
+      )
+      .reduce((prev, child) => {
+        return [
+          ...prev,
+          ...getRowGroupElementsInElement(child, tagName === "table"),
+        ];
+      }, [] as Element[]);
   }
   return [];
 };
@@ -527,3 +552,6 @@ export const getCellElements = (el: Element): Element[] => {
   }
   return [];
 };
+
+export const isEmptyCellElement = (el: Element): boolean =>
+  el.children.length === 0 && (!el.textContent || el.textContent.trim() === "");


Test patch:
diff --git a/src/content/dom/tips/internalTable.test.ts b/src/content/dom/tips/internalTable.test.ts
index 3033ba8..af78494 100644
--- a/src/content/dom/tips/internalTable.test.ts
+++ b/src/content/dom/tips/internalTable.test.ts
@@ -4,6 +4,7 @@ import {
   getRowElements,
   getRowGroupElements,
   getCellElements,
+  isEmptyCellElement,
 } from "./internalTable";
 
 describe("InternalTable", () => {
@@ -77,10 +78,28 @@ describe("InternalTable", () => {
     table.appendChild(document.createElement("tr"));
     const result = new InternalTable(table);
     const { rowGroups, cells } = result;
-    expect(rowGroups[0].positionY).toBe(0);
-    expect(rowGroups[0].sizeY).toBe(3);
-    expect(rowGroups[0].element).toBeNull();
+    expect(rowGroups).toHaveLength(0);
+    expect(cells).toHaveLength(3);
+  });
+
+  test("wrong tfoot position", () => {
+    const table = document.createElement("table");
+    table.innerHTML = `
+      <caption>caption</caption>
+      <tfoot>
+        <tr><td id="cell-2-0">2-0</td></tr>
+      </tfoot>
+      <tbody>
+        <tr><td id="cell-0-0">0-0</td></tr>
+        <tr><td id="cell-1-0">1-0</td></tr>
+      </tbody>
+    `;
+    const result = new InternalTable(table);
+    const { cells } = result;
     expect(cells).toHaveLength(3);
+    expect(cells[0][0].element.id).toBe("cell-0-0");
+    expect(cells[1][0].element.id).toBe("cell-1-0");
+    expect(cells[2][0].element.id).toBe("cell-2-0");
   });
 
   test("colgroups", () => {
@@ -94,6 +113,7 @@ describe("InternalTable", () => {
       <col></col>
       <col></col>
     </colgroup>
+    <colgroup id="3-8"></colgroup>
     <tbody>
       <tr>
         <td>0</td><td>1</td><td>2</td><td>3</td><td>4</td>
@@ -114,9 +134,8 @@ describe("InternalTable", () => {
     expect(colGroups[1].sizeX).toBe(3);
     expect(colGroups[2].positionX).toBe(4);
     expect(colGroups[2].sizeX).toBe(4);
-    expect(colGroups[3]).toBeDefined();
     expect(colGroups[3].positionX).toBe(8);
-    expect(colGroups[3].sizeX).toBe(2);
+    expect(colGroups[3].sizeX).toBe(1);
     expect(colGroups).toHaveLength(4);
   });
 
@@ -611,25 +630,50 @@ describe("getRowElements", () => {
     table.innerHTML = `
       <caption>caption</caption>
       <thead>
-        <tr><td>thead row 0</td></tr>
+        <tr id="row-0"><td>thead row 0</td></tr>
       </thead>
       <tbody>
-        <tr><td>tbody0 row 0</td></tr>
-        <tr><td>tbody0 row 1</td></tr>
+        <tr id="row-1"><td>tbody0 row 0</td></tr>
+        <tr id="row-2"><td>tbody0 row 1</td></tr>
       </tbody>
       <tbody>
-        <tr><td>tbody1 row 0</td></tr>
+        <tr id="row-3"><td>tbody1 row 0</td></tr>
       </tbody>
       <tfoot>
-        <tr><td>tfoot row 0</td></tr>
+        <tr id="row-4"><td>tfoot row 0</td></tr>
       </tfoot>
     `;
     const result = getRowElements(table);
-    result.forEach((row) => {
+    result.forEach((row, i) => {
       expect(row.tagName.toLowerCase()).toBe("tr");
+      expect(row.id).toBe(`row-${i}`);
     });
     expect(result).toHaveLength(5);
   });
+
+  test("wrong tfoot position", () => {
+    const table = document.createElement("table");
+    table.innerHTML = `
+      <caption>caption</caption>
+      <tfoot>
+        <tr id="row-2"><td id="cell-2-0">2-0</td></tr>
+      </tfoot>
+      <tbody>
+        <tr id="row-0"><td>0-0</td></tr>
+        <tr id="row-1"><td>1-0</td></tr>
+      </tbody>
+      <tfoot>
+        <tr id="row-3"><td>3-0</td></tr>
+      </tfoot>
+    `;
+    const result = getRowElements(table);
+    result.forEach((row, i) => {
+      expect(row.tagName.toLowerCase()).toBe("tr");
+      expect(row.id).toBe(`row-${i}`);
+    });
+    expect(result).toHaveLength(4);
+  });
+
   test("table has directly added tr", () => {
     const table = document.createElement("table");
     table.appendChild(document.createElement("tr"));
@@ -725,6 +769,31 @@ describe("getRowGroupElements", () => {
     expect(result).toHaveLength(4);
   });
 
+  test("wrong tfoot position", () => {
+    const table = document.createElement("table");
+    table.innerHTML = `
+      <caption>caption</caption>
+      <tfoot id="group-2">
+        <tr><td>2-0</td></tr>
+      </tfoot>
+      <thead id="group-0">
+        <tr><td>0-0</td></tr>
+      </thead>
+      <tbody id="group-1">
+        <tr><td>1-0</td></tr>
+      </tbody>
+      <tfoot id="group-3">
+        <tr><td>3-0</td></tr>
+      </tfoot>
+    `;
+    const result = getRowGroupElements(table);
+    expect(result[0].tagName.toLowerCase()).toBe("thead");
+    expect(result[1].tagName.toLowerCase()).toBe("tbody");
+    expect(result[2].tagName.toLowerCase()).toBe("tfoot");
+    expect(result[3].tagName.toLowerCase()).toBe("tfoot");
+    expect(result).toHaveLength(4);
+  });
+
   test("table has directly added tr", () => {
     const table = document.createElement("table");
     table.appendChild(document.createElement("tr"));
@@ -823,3 +892,30 @@ describe("getCellElements", () => {
     expect(result).toHaveLength(12);
   });
 });
+
+describe("isEmptyCellElement", () => {
+  test("empty cell", () => {
+    const cell = document.createElement("td");
+    expect(isEmptyCellElement(cell)).toBe(true);
+  });
+  test("cell with text", () => {
+    const cell = document.createElement("td");
+    cell.textContent = "text";
+    expect(isEmptyCellElement(cell)).toBe(false);
+  });
+  test("cell with space", () => {
+    const cell = document.createElement("td");
+    cell.textContent = " ";
+    expect(isEmptyCellElement(cell)).toBe(true);
+  });
+  test("cell with br", () => {
+    const cell = document.createElement("td");
+    cell.appendChild(document.createElement("br"));
+    expect(isEmptyCellElement(cell)).toBe(false);
+  });
+  test("cell with img", () => {
+    const cell = document.createElement("td");
+    cell.appendChild(document.createElement("img"));
+    expect(isEmptyCellElement(cell)).toBe(false);
+  });
+});
diff --git a/src/test/TestPage.tsx b/src/test/TestPage.tsx
index ee31517..7b56fde 100644
--- a/src/test/TestPage.tsx
+++ b/src/test/TestPage.tsx
@@ -8,6 +8,7 @@ import {
   Layout,
   LiveRegion,
 } from "./categories";
+import { Table } from "./categories/Table";
 
 export const TestPage = () => (
   <main className="p-3">
@@ -24,6 +25,7 @@ export const TestPage = () => (
     <Link />
     <FormControl />
     <Heading />
+    <Table />
     <AriaHidden />
     <Layout />
     <LiveRegion />
diff --git a/src/test/categories/Table.tsx b/src/test/categories/Table.tsx
new file mode 100644
index 0000000..d63f48d
--- /dev/null
+++ b/src/test/categories/Table.tsx
@@ -0,0 +1,89 @@
+import { CategoryTitle, ExampleList } from "../components";
+
+export const Table = () => (
+  <>
+    <CategoryTitle>Tables</CategoryTitle>
+    <ExampleList
+      examples={[
+        {
+          title: "Table with header and footer",
+          body: (
+            <table className="table-auto border-2">
+              <caption>Table with header and footer</caption>
+              <thead>
+                <tr>
+                  <th className="p-2 border-2">0-0</th>
+                  <th className="p-2 border-2">0-1</th>
+                  <th className="p-2 border-2">0-2</th>
+                </tr>
+              </thead>
+              <tbody>
+                <tr>
+                  <td className="p-2 border-2">1-0</td>
+                  <td className="p-2 border-2">1-1</td>
+                  <td className="p-2 border-2">1-2</td>
+                </tr>
+                <tr>
+                  <td className="p-2 border-2">2-0</td>
+                  <td className="p-2 border-2">2-1</td>
+                  <td className="p-2 border-2">2-2</td>
+                </tr>
+                <tr>
+                  <td className="p-2 border-2">3-0</td>
+                  <td className="p-2 border-2">3-1</td>
+                  <td className="p-2 border-2">3-2</td>
+                </tr>
+              </tbody>
+              <tfoot>
+                <tr>
+                  <td className="p-2 border-2">4-0</td>
+                  <td className="p-2 border-2">4-1</td>
+                  <td className="p-2 border-2">4-2</td>
+                </tr>
+              </tfoot>
+            </table>
+          ),
+        },
+        {
+          title: "Wrong tfoot position",
+          body: (
+            <table className="table-auto border-2 intentional-violation">
+              <caption>Wrong tfoot position table</caption>
+              <thead>
+                <tr>
+                  <th className="p-2 border-2">0-0</th>
+                  <th className="p-2 border-2">0-1</th>
+                  <th className="p-2 border-2">0-2</th>
+                </tr>
+              </thead>
+              <tfoot className="intentional-violation">
+                <tr>
+                  <td className="p-2 border-2">4-0</td>
+                  <td className="p-2 border-2">4-1</td>
+                  <td className="p-2 border-2">4-2</td>
+                </tr>
+              </tfoot>
+              <tbody className="intentional-violation">
+                <tr>
+                  <td className="p-2 border-2">1-0</td>
+                  <td className="p-2 border-2">1-1</td>
+                  <td className="p-2 border-2">1-2</td>
+                </tr>
+                <tr>
+                  <td className="p-2 border-2">2-0</td>
+                  <td className="p-2 border-2">2-1</td>
+                  <td className="p-2 border-2">2-2</td>
+                </tr>
+                <tr>
+                  <td className="p-2 border-2">3-0</td>
+                  <td className="p-2 border-2">3-1</td>
+                  <td className="p-2 border-2">3-2</td>
+                </tr>
+              </tbody>
+            </table>
+          ),
+        },
+      ]}
+    />
+  </>
+);