diff --git a/src/Content/classicDesktopFrame.css b/src/Content/classicDesktopFrame.css index 5216e202..1663a458 100644 --- a/src/Content/classicDesktopFrame.css +++ b/src/Content/classicDesktopFrame.css @@ -99,24 +99,58 @@ td { background-color: #CDE6F7; /* 2013 light background color */ } -.tableHeaderButton { +.sortableHeaderCell { background-color: #0072C6; /*2013 hero color*/ color: #FFFFFF; border: 0; font-weight: bold; - width: 100%; + width: auto; cursor: pointer; padding: 1em 1.2em 1em 1.2em; + white-space: nowrap; + line-height: 1.2; + text-align: center; + vertical-align: middle; +} + +#response th.sortableHeaderCell { + padding: 1em 1.2em 1em 1.2em; +} + +.sortableHeaderCell .headerLabel { + display: inline; + vertical-align: middle; } -th[aria-sort="descending"] .tableHeaderButton .sortArrow::after { +.sortableHeaderCell .sortArrow { + display: inline; + min-width: 0; + margin-left: 2px; + vertical-align: middle; +} + +.sortableHeaderCell:focus-visible { + outline: black solid 2px; + outline-offset: -2px; +} + +.sortableHeaderCell:focus { + outline: black solid 2px; + outline-offset: -2px; +} + +th[aria-sort="descending"] .sortArrow::after { content: "\2193"; /* Down arrow */ } -th[aria-sort="ascending"] .tableHeaderButton .sortArrow::after { +th[aria-sort="ascending"] .sortArrow::after { content: "\2191"; /* Up arrow */ } +th[aria-sort="none"] .sortArrow::after { + content: ""; +} + .tableHeader td + td, .tableHeader th + th { border-left: 1px solid white; } .tableHeader tr + tr { border-top: 1px solid black; } @@ -268,11 +302,11 @@ button[aria-expanded="false"] .collapsibleSwitch::after { } } -#hop { +#hop_header { min-width: 3em; } -#number { +#number_header { min-width: 2em; } @@ -299,7 +333,7 @@ button[aria-expanded="false"] .collapsibleSwitch::after { } @media screen and (-ms-high-contrast:active) { - .tableHeaderButton .sortArrow::after { color: buttontext; } + .sortableHeaderCell .sortArrow::after { color: buttontext; } button .collapsibleSwitch::after { color: buttontext; } } diff --git a/src/Scripts/rules/ViolationUtils.test.ts b/src/Scripts/rules/ViolationUtils.test.ts index 3d040ea2..e2b3c221 100644 --- a/src/Scripts/rules/ViolationUtils.test.ts +++ b/src/Scripts/rules/ViolationUtils.test.ts @@ -561,6 +561,77 @@ describe("getViolationsForRow", () => { expect(result).toHaveLength(1); }); + + test("should not cross-match source rows from different header families", () => { + const ffasSection: HeaderSection = { + header: "source", + headerName: "X-Forefront-Antispam-Report", + value: "SFV:SPM;" + }; + const asSection: HeaderSection = { + header: "source", + headerName: "X-Microsoft-Antispam", + value: "BCL:8;" + }; + + const ffasRule = new SimpleValidationRule( + "X-Forefront-Antispam-Report", + "SFV:SPM", + "Message was marked as spam", + ["source"], + "error" + ); + const asRule = new SimpleValidationRule( + "X-Microsoft-Antispam", + "BCL:[6789]", + "Bulk Sender Reputation is bad", + ["source"], + "error" + ); + + const ffasViolation: RuleViolation = { + rule: ffasRule, + affectedSections: [ffasSection], + highlightPattern: "SFV:SPM" + }; + const asViolation: RuleViolation = { + rule: asRule, + affectedSections: [asSection], + highlightPattern: "BCL:[6789]" + }; + + const group: ViolationGroup = { + groupId: "group-1", + displayName: "Source violations", + severity: "error", + isAndRule: false, + violations: [ffasViolation, asViolation] + }; + + const ffasRow = { + header: "source", + headerName: "X-Forefront-Antispam-Report", + label: "Source header", + value: "SFV:SPM;" + }; + const asRow = { + header: "source", + headerName: "X-Microsoft-Antispam", + label: "Source header", + value: "BCL:8;" + }; + + const ffasResults = getViolationsForRow(ffasRow, [group]); + const asResults = getViolationsForRow(asRow, [group]); + + expect(ffasResults).toHaveLength(1); + expect(ffasResults).toContain(ffasViolation); + expect(ffasResults).not.toContain(asViolation); + + expect(asResults).toHaveLength(1); + expect(asResults).toContain(asViolation); + expect(asResults).not.toContain(ffasViolation); + }); }); describe("highlightHtml", () => { diff --git a/src/Scripts/rules/ViolationUtils.ts b/src/Scripts/rules/ViolationUtils.ts index 877f9c13..d9aa741c 100644 --- a/src/Scripts/rules/ViolationUtils.ts +++ b/src/Scripts/rules/ViolationUtils.ts @@ -158,10 +158,27 @@ export function getViolationsForRow( // Check if violation applies to this row via any of its affected sections const matchesSection = violation.affectedSections.some(section => { const headerSection = section as HeaderSection; - // Match by section header/name or headerName property - return headerSection.header === row.label || - headerSection.header === row.header || - headerSection.header === row.headerName; + + // Prefer exact header/headerName matching to avoid collisions where + // different tables share generic row headers like "source". + if (row.header && headerSection.header === row.header) { + if (row.headerName && headerSection.headerName) { + return headerSection.headerName === row.headerName; + } + + if (row.headerName && !headerSection.headerName) { + return false; + } + + return true; + } + + // Fallback for rows without explicit header metadata. + if (row.label && headerSection.header === row.label) { + return true; + } + + return false; }); if (matchesSection) { diff --git a/src/Scripts/rules/engine/HeaderValidationRules.test.ts b/src/Scripts/rules/engine/HeaderValidationRules.test.ts index 8a086d2e..099b6307 100644 --- a/src/Scripts/rules/engine/HeaderValidationRules.test.ts +++ b/src/Scripts/rules/engine/HeaderValidationRules.test.ts @@ -601,4 +601,29 @@ describe("findSectionSubSection", () => { expect(results).toHaveLength(1); expect(results[0]!.value).toBe("Test"); }); + + test("should not match non-source rows by headerName", () => { + const sections: HeaderSection[][] = [ + [ + { header: "SFV", value: "SPM", headerName: "X-Forefront-Antispam-Report" }, + { header: "CTRY", value: "US", headerName: "X-Forefront-Antispam-Report" } + ] + ]; + + const results = findSectionSubSection(sections, "X-Forefront-Antispam-Report"); + expect(results).toHaveLength(0); + }); + + test("should match source row by headerName", () => { + const sections: HeaderSection[][] = [ + [ + { header: "source", value: "SFV:SPM;CTRY:US;", headerName: "X-Forefront-Antispam-Report" }, + { header: "SFV", value: "SPM", headerName: "X-Forefront-Antispam-Report" } + ] + ]; + + const results = findSectionSubSection(sections, "X-Forefront-Antispam-Report"); + expect(results).toHaveLength(1); + expect(results[0]!.header).toBe("source"); + }); }); diff --git a/src/Scripts/rules/engine/HeaderValidationRules.ts b/src/Scripts/rules/engine/HeaderValidationRules.ts index d2633c27..003cd640 100644 --- a/src/Scripts/rules/engine/HeaderValidationRules.ts +++ b/src/Scripts/rules/engine/HeaderValidationRules.ts @@ -278,7 +278,13 @@ export function findSectionSubSection(setOfSections: HeaderSection[][], subSecti setOfSections.forEach((section) => { section.forEach((subSection) => { - if (subSection.header === subSectionLookingFor || subSection.headerName === subSectionLookingFor) { + // Match explicit sections by header. Only allow headerName matches for + // the source row so rules can still target full raw header content + // without fanning out to every parsed child row. + const headerMatch = subSection.header === subSectionLookingFor; + const sourceHeaderMatch = subSection.headerName === subSectionLookingFor && subSection.header === "source"; + + if (headerMatch || sourceHeaderMatch) { results.push(subSection); } }); diff --git a/src/Scripts/ui/Table.test.ts b/src/Scripts/ui/Table.test.ts index e1c80fea..3f2f36f3 100644 --- a/src/Scripts/ui/Table.test.ts +++ b/src/Scripts/ui/Table.test.ts @@ -156,19 +156,14 @@ describe("Table", () => { }); it("should reset arrows", () => { - const by = document.querySelector("#receivedHeaders th #by") as HTMLElement; - const hop = document.querySelector("#receivedHeaders th #hop") as HTMLElement; - const number = document.querySelector("#otherHeaders th #number") as HTMLElement; - - // Check aria-sort on the parent th elements instead of buttons - const byTh = by?.parentElement as HTMLElement; - const hopTh = hop?.parentElement as HTMLElement; - const numberTh = number?.parentElement as HTMLElement; + const byTh = document.querySelector("#receivedHeaders th#by_header") as HTMLElement; + const hopTh = document.querySelector("#receivedHeaders th#hop_header") as HTMLElement; + const numberTh = document.querySelector("#otherHeaders th#number_header") as HTMLElement; expect(hopTh.getAttribute("aria-sort")).toBe("descending"); expect(byTh.getAttribute("aria-sort")).toBe("none"); expect(numberTh.getAttribute("aria-sort")).toBe("descending"); - by.click(); + byTh.click(); expect(hopTh.getAttribute("aria-sort")).toBe("none"); expect(byTh.getAttribute("aria-sort")).toBe("descending"); table.resetArrows(); @@ -176,6 +171,28 @@ describe("Table", () => { expect(byTh.getAttribute("aria-sort")).toBe("none"); }); + it("should support keyboard sorting on header cells", () => { + const byTh = document.querySelector("#receivedHeaders th#by_header") as HTMLElement; + const hopTh = document.querySelector("#receivedHeaders th#hop_header") as HTMLElement; + + expect(byTh.getAttribute("tabindex")).toBe("0"); + expect(byTh.getAttribute("aria-label")).toContain("Sort by"); + + const enterKeyDown = new KeyboardEvent("keydown", { key: "Enter", bubbles: true }); + byTh.dispatchEvent(enterKeyDown); + + const firstSortDirection = byTh.getAttribute("aria-sort"); + expect(firstSortDirection === "descending" || firstSortDirection === "ascending").toBe(true); + expect(hopTh.getAttribute("aria-sort")).toBe("none"); + + const spaceKeyDown = new KeyboardEvent("keydown", { key: " ", bubbles: true }); + byTh.dispatchEvent(spaceKeyDown); + + const secondSortDirection = byTh.getAttribute("aria-sort"); + expect(secondSortDirection === "descending" || secondSortDirection === "ascending").toBe(true); + expect(secondSortDirection).not.toBe(firstSortDirection); + }); + describe("TableSection Integration", () => { it("should initialize table sections with proper accessibility", () => { // Check that table sections exist on viewModel (some may be undefined if no data) diff --git a/src/Scripts/ui/Table.ts b/src/Scripts/ui/Table.ts index adaa9996..8fdeef60 100644 --- a/src/Scripts/ui/Table.ts +++ b/src/Scripts/ui/Table.ts @@ -151,9 +151,19 @@ export class Table { const headers = document.querySelectorAll(`#${table} th[role="columnheader"]`); headers.forEach(header => header.setAttribute("aria-sort", "none")); - const targetButton = document.querySelector(`#${table} th #${colName}`); - if (targetButton && targetButton.parentElement) { - targetButton.parentElement.setAttribute("aria-sort", sortOrder === 1 ? "descending" : "ascending"); + const targetHeader = document.querySelector(`#${table} th#${colName}_header`); + if (targetHeader) { + targetHeader.setAttribute("aria-sort", sortOrder === 1 ? "descending" : "ascending"); + } + } + + private sortByColumn(tableName: string, columnId: string): void { + if (this.viewModel && this.viewModel[tableName] instanceof DataTable) { + const dataTable = this.viewModel[tableName] as DataTable; + dataTable.doSort(columnId); + this.setArrows(dataTable.tableName, dataTable.sortColumn, + dataTable.sortOrder); + this.rebuildSections(this.viewModel); } } @@ -367,34 +377,35 @@ export class Table { header.setAttribute("aria-sort", "none"); header.setAttribute("id", column.id + "_header"); // Add unique ID for headers attribute - const headerButton = document.createElement("button"); - if (headerButton !== null) { - headerButton.setAttribute("class", "tableHeaderButton"); - headerButton.setAttribute("id", column.id); - headerButton.setAttribute("type", "button"); - headerButton.innerHTML = column.label; - if (column.class !== null) { - headerButton.setAttribute("class", "tableHeaderButton " + column.class); - } + header.classList.add("sortableHeaderCell"); + if (column.class) { + header.classList.add(column.class); + } - headerButton.addEventListener("click", () => { - if (this.viewModel && this.viewModel[tableName] instanceof DataTable) { - const dataTable = this.viewModel[tableName] as DataTable; - dataTable.doSort(column.id); - this.setArrows(dataTable.tableName, dataTable.sortColumn, - dataTable.sortOrder); - this.rebuildSections(this.viewModel); - } - }); + const labelSpan = document.createElement("span"); + labelSpan.classList.add("headerLabel"); + labelSpan.textContent = column.label; - const arrowSpan = document.createElement("span"); - arrowSpan.setAttribute("aria-hidden", "true"); - arrowSpan.classList.add("sortArrow"); + const arrowSpan = document.createElement("span"); + arrowSpan.setAttribute("aria-hidden", "true"); + arrowSpan.classList.add("sortArrow"); - // Now that everything is built, put it together - headerButton.appendChild(arrowSpan); - header.appendChild(headerButton); - } + header.appendChild(labelSpan); + header.appendChild(arrowSpan); + + header.setAttribute("tabindex", "0"); + header.setAttribute("aria-label", `Sort by ${column.label}`); + + header.addEventListener("click", () => { + this.sortByColumn(tableName, column.id); + }); + + header.addEventListener("keydown", (event: KeyboardEvent) => { + if (event.key === "Enter" || event.key === " ") { + event.preventDefault(); + this.sortByColumn(tableName, column.id); + } + }); } return header; @@ -496,7 +507,7 @@ export class Table { } private setupReceivedHeadersUI(): void { - const withColumn = document.querySelector("#receivedHeaders #with"); + const withColumn = document.querySelector("#receivedHeaders #with_header"); if (withColumn !== null) { if (document.getElementById("leftArrow")) return; // Skip if already exists