From 14ac6b7aa5591fe532b70d3c19ee6c066d53e671 Mon Sep 17 00:00:00 2001 From: Gary Ritchie Date: Sun, 18 Oct 2026 11:47:24 -0600 Subject: [PATCH 1/8] Expose DataGrid header and body tables as one ARIA table DataGrid renders its header, body and footer as three separate elements so the header can stay fixed while the body scrolls, which assistive technology reads as a headers-only table and a headerless data table. DataGrid now carries role="table", its three inner tables and the empty/loading containers are presentational, and each section is a rowgroup. A new protected hook, AbstractCellTable.onSectionRendered, runs after any head, body or foot rows are replaced (full or partial redraw); DataGrid uses it to give rows role="row" and cells columnheader, rowheader or cell. Rows that already carry a role other than row, such as layout rows marked presentation, are left alone. Addresses rstudio/rstudio#19173 --- .../cellview/client/AbstractCellTable.java | 11 ++ .../gwt/user/cellview/client/DataGrid.java | 54 ++++++- .../client/AbstractCellTableTestBase.java | 12 +- .../user/cellview/client/DataGridTest.java | 132 ++++++++++++++++++ 4 files changed, 205 insertions(+), 4 deletions(-) diff --git a/user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java b/user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java index 59738224886..81b3a086dc0 100644 --- a/user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java +++ b/user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java @@ -529,6 +529,7 @@ public final void replaceAllRows(AbstractCellTable table, TableSectionElement * appended instead. */ reattachSectionElement(parent, section, nextSection); + table.onSectionRendered(section); // Detach the event listener. if (!table.isAttached()) { @@ -597,6 +598,7 @@ public final void replaceChildren(AbstractCellTable table, TableSectionElemen * appended instead. */ reattachSectionElement(parent, section, nextSection); + table.onSectionRendered(section); // Detach the event listener. if (!table.isAttached()) { @@ -1706,6 +1708,15 @@ protected boolean dependsOnSelection() { */ protected abstract void doSetHeaderVisible(boolean isFooter, boolean isVisible); + /** + * Called after the rows of a head, body or foot section are replaced, so + * subclasses can post-process the rendered rows. The default does nothing. + * + * @param section the section whose rows were just rendered + */ + protected void onSectionRendered(TableSectionElement section) { + } + @Override protected Element getChildContainer() { return getTableBodyElement(); diff --git a/user/src/com/google/gwt/user/cellview/client/DataGrid.java b/user/src/com/google/gwt/user/cellview/client/DataGrid.java index 47bf14f9894..8f6dca4e9ed 100644 --- a/user/src/com/google/gwt/user/cellview/client/DataGrid.java +++ b/user/src/com/google/gwt/user/cellview/client/DataGrid.java @@ -18,12 +18,15 @@ import com.google.gwt.core.client.GWT; import com.google.gwt.dom.client.Document; import com.google.gwt.dom.client.Element; +import com.google.gwt.dom.client.NodeList; import com.google.gwt.dom.client.Style.Display; import com.google.gwt.dom.client.Style.Overflow; import com.google.gwt.dom.client.Style.TableLayout; import com.google.gwt.dom.client.Style.Unit; +import com.google.gwt.dom.client.TableCellElement; import com.google.gwt.dom.client.TableColElement; import com.google.gwt.dom.client.TableElement; +import com.google.gwt.dom.client.TableRowElement; import com.google.gwt.dom.client.TableSectionElement; import com.google.gwt.event.dom.client.ScrollEvent; import com.google.gwt.event.dom.client.ScrollHandler; @@ -504,12 +507,40 @@ private static Resources getDefaultResources() { return DEFAULT_RESOURCES; } + /** + * Give the rows and cells of a rendered section explicit ARIA roles, which + * they lose to the presentation role on their table. Rows and cells that + * already have a role keep it, and a row with a role other than row (such + * as a layout row marked presentation) is left alone, cells included. + */ + private static void applyTableRoles(TableSectionElement section) { + boolean isBody = "tbody".equalsIgnoreCase(section.getTagName()); + NodeList rows = section.getRows(); + for (int r = 0; r < rows.getLength(); r++) { + TableRowElement row = rows.getItem(r); + if (!row.hasAttribute("role")) { + row.setAttribute("role", "row"); + } else if (!"row".equals(row.getAttribute("role"))) { + continue; + } + NodeList cells = row.getCells(); + for (int c = 0; c < cells.getLength(); c++) { + TableCellElement cell = cells.getItem(c); + if (cell.hasAttribute("role")) { + continue; + } + boolean isTh = "th".equalsIgnoreCase(cell.getTagName()); + cell.setAttribute("role", isTh ? (isBody ? "rowheader" : "columnheader") : "cell"); + } + } + } + final TableWidget tableData; final TableWidget tableFooter; final TableWidget tableHeader; - private final FlexTable emptyTableWidgetContainer; + final FlexTable emptyTableWidgetContainer; private final HeaderPanel headerPanel; - private final FlexTable loadingIndicatorContainer; + final FlexTable loadingIndicatorContainer; private final Style style; private final Element tableDataContainer; private final ScrollPanel tableDataScroller; @@ -649,6 +680,20 @@ public DataGrid(int pageSize, Resources resources, ProvidesKey keyProvider, loadingIndicatorContainer = new FlexTable(); loadingIndicatorContainer.getElement().setAttribute("align", "center"); + /* + * The header, body and footer are separate tables so the header can stay + * fixed while the body scrolls. Presentation roles on those tables, plus + * explicit row and cell roles (see onSectionRendered), let assistive + * technology read them as one table. + */ + getElement().setAttribute("role", "table"); + for (TableWidget part : new TableWidget[] {tableHeader, tableData, tableFooter}) { + part.tableElem.setAttribute("role", "presentation"); + part.section.setAttribute("role", "rowgroup"); + } + emptyTableWidgetContainer.getElement().setAttribute("role", "presentation"); + loadingIndicatorContainer.getElement().setAttribute("role", "presentation"); + // Set the loading indicator. setLoadingIndicator(loadingIndicator); // Can be null. @@ -858,6 +903,11 @@ protected void onLoadingStateChanged(LoadingState state) { super.onLoadingStateChanged(state); } + @Override + protected void onSectionRendered(TableSectionElement section) { + applyTableRoles(section); + } + @Override protected void refreshColumnWidths() { super.refreshColumnWidths(); diff --git a/user/test/com/google/gwt/user/cellview/client/AbstractCellTableTestBase.java b/user/test/com/google/gwt/user/cellview/client/AbstractCellTableTestBase.java index 45f4336703c..c3479a1efb1 100644 --- a/user/test/com/google/gwt/user/cellview/client/AbstractCellTableTestBase.java +++ b/user/test/com/google/gwt/user/cellview/client/AbstractCellTableTestBase.java @@ -1224,7 +1224,7 @@ public void testSortableHeaderAria() { // The sortable th keeps its columnheader role; the button is inside it. TableCellElement sortableTh = getHeaderElement(table, 0); - assertFalse(sortableTh.hasAttribute("role")); + assertEquals(getHeaderCellRole(), sortableTh.getAttribute("role")); assertFalse(sortableTh.hasAttribute("aria-sort")); Element sortButton = sortableTh.getFirstChildElement(); assertEquals("button", sortButton.getAttribute("role")); @@ -1233,7 +1233,7 @@ public void testSortableHeaderAria() { // A column that isn't sortable gets no button. TableCellElement plainTh = getHeaderElement(table, 1); - assertFalse(plainTh.hasAttribute("role")); + assertEquals(getHeaderCellRole(), plainTh.getAttribute("role")); assertFalse(plainTh.hasAttribute("aria-sort")); assertNull(plainTh.getFirstChildElement()); assertEquals("Column 1", plainTh.getInnerText()); @@ -1374,6 +1374,14 @@ public String getValue(String object) { return table; } + /** + * Get the role attribute a header th should carry: empty for an implicit + * columnheader, or the explicit role the table stamps on it. + */ + protected String getHeaderCellRole() { + return ""; + } + /** * Get a td element from the table body. * diff --git a/user/test/com/google/gwt/user/cellview/client/DataGridTest.java b/user/test/com/google/gwt/user/cellview/client/DataGridTest.java index f3515e9a968..a778548e065 100644 --- a/user/test/com/google/gwt/user/cellview/client/DataGridTest.java +++ b/user/test/com/google/gwt/user/cellview/client/DataGridTest.java @@ -15,14 +15,19 @@ */ package com.google.gwt.user.cellview.client; +import com.google.gwt.cell.client.TextCell; import com.google.gwt.core.client.GWT; +import com.google.gwt.dom.builder.shared.TableRowBuilder; import com.google.gwt.dom.client.TableCellElement; import com.google.gwt.dom.client.TableElement; import com.google.gwt.dom.client.TableRowElement; import com.google.gwt.dom.client.TableSectionElement; import com.google.gwt.user.cellview.client.DataGrid.Resources; +import com.google.gwt.user.client.ui.Label; import com.google.gwt.user.client.ui.RootPanel; +import java.util.Arrays; + /** * Tests for {@link DataGrid}. */ @@ -89,11 +94,138 @@ protected boolean buildHeaderOrFooterImpl() { RootPanel.get().remove(table); } + /** + * Test that the header, body and footer tables read as one ARIA table. + */ + public void testTableRoles() { + DataGrid table = createRoleTestGrid(); + + assertEquals("table", table.getElement().getAttribute("role")); + assertEquals("presentation", table.tableHeader.getElement().getAttribute("role")); + assertEquals("presentation", table.tableData.getElement().getAttribute("role")); + assertEquals("presentation", table.tableFooter.getElement().getAttribute("role")); + assertEquals("rowgroup", table.getTableHeadElement().getAttribute("role")); + assertEquals("rowgroup", table.getTableBodyElement().getAttribute("role")); + assertEquals("rowgroup", table.getTableFootElement().getAttribute("role")); + + TableRowElement headerRow = table.getTableHeadElement().getRows().getItem(0); + assertEquals("row", headerRow.getAttribute("role")); + for (int c = 0; c < 3; c++) { + assertEquals("columnheader", headerRow.getCells().getItem(c).getAttribute("role")); + } + for (int r = 0; r < 3; r++) { + TableRowElement row = table.getRowElement(r); + assertEquals("row", row.getAttribute("role")); + for (int c = 0; c < 3; c++) { + assertEquals("cell", row.getCells().getItem(c).getAttribute("role")); + } + } + + RootPanel.get().remove(table); + } + + /** + * Test that rows replaced by a partial redraw get roles too. + */ + public void testTableRolesAfterPartialRedraw() { + DataGrid table = createRoleTestGrid(); + + table.setRowData(1, Arrays.asList("B")); + table.getPresenter().flush(); + TableRowElement row = table.getRowElement(1); + assertEquals("B", row.getCells().getItem(0).getInnerText()); + assertEquals("row", row.getAttribute("role")); + assertEquals("cell", row.getCells().getItem(1).getAttribute("role")); + + RootPanel.get().remove(table); + } + + /** + * Test that a row the builder marks presentational keeps that role and its + * cells get none. + */ + public void testTableRolesKeepExplicitRoles() { + DataGrid table = createRoleTestGrid(); + + table.setTableBuilder(new DefaultCellTableBuilder(table) { + @Override + public void buildRowImpl(String rowValue, int absRowIndex) { + super.buildRowImpl(rowValue, absRowIndex); + if (absRowIndex == 0) { + TableRowBuilder tr = startRow(rowValue); + tr.attribute("role", "presentation"); + tr.startTD().colSpan(3).endTD(); + tr.endTR(); + } + } + }); + table.getPresenter().flush(); + TableRowElement layoutRow = table.getTableBodyElement().getRows().getItem(1); + assertEquals("presentation", layoutRow.getAttribute("role")); + assertFalse(layoutRow.getCells().getItem(0).hasAttribute("role")); + assertEquals("row", table.getTableBodyElement().getRows().getItem(2).getAttribute("role")); + + RootPanel.get().remove(table); + } + + /** + * Test that a sorted header carries both its role and aria-sort. + */ + public void testTableRolesKeepAriaSort() { + DataGrid table = createRoleTestGrid(); + + table.getColumnSortList().push(table.getColumn(2)); + table.redrawHeaders(); + TableCellElement th = table.getTableHeadElement().getRows().getItem(0).getCells().getItem(2); + assertEquals("columnheader", th.getAttribute("role")); + assertEquals("ascending", th.getAttribute("aria-sort")); + + RootPanel.get().remove(table); + } + + /** + * Test that the empty-table and loading containers do not read as tables. + */ + public void testEmptyAndLoadingContainersArePresentational() { + DataGrid table = createAbstractHasData(); + table.setEmptyTableWidget(new Label("Empty")); + table.setLoadingIndicator(new Label("Loading")); + assertEquals("presentation", + table.emptyTableWidgetContainer.getElement().getAttribute("role")); + assertEquals("presentation", + table.loadingIndicatorContainer.getElement().getAttribute("role")); + } + @Override protected DataGrid createAbstractHasData() { return new DataGrid(); } + @Override + protected String getHeaderCellRole() { + return "columnheader"; + } + + /** + * Create an attached grid with three columns, the last one sortable, and + * three rows. + */ + private DataGrid createRoleTestGrid() { + DataGrid table = createAbstractHasData(new TextCell()); + TextColumn column = new TextColumn() { + @Override + public String getValue(String object) { + return object; + } + }; + column.setSortable(true); + table.addColumn(column, new TextHeader("Name")); + RootPanel.get().add(table); + table.setRowData(Arrays.asList("a", "b", "c")); + table.getPresenter().flush(); + return table; + } + @Override protected TableCellElement getBodyElement(DataGrid table, int row, int column) { TableElement tableElem = table.tableData.getElement().cast(); From 7135de24ca3baeb519104d227f01b54ab09d0436 Mon Sep 17 00:00:00 2001 From: Gary Ritchie Date: Sun, 18 Oct 2026 12:34:47 -0600 Subject: [PATCH 2/8] Apply DataGrid row roles before the section is reattached replaceAllRows and replaceChildren detach the section while they render into it. Calling onSectionRendered before reattaching it keeps the role writes off the live document, so the accessibility tree is built once with the final roles. Addresses rstudio/rstudio#19173 --- .../gwt/user/cellview/client/AbstractCellTable.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java b/user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java index 81b3a086dc0..998dce98b3b 100644 --- a/user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java +++ b/user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java @@ -528,8 +528,8 @@ public final void replaceAllRows(AbstractCellTable table, TableSectionElement * Reattach the section. If next section is null, the section will be * appended instead. */ - reattachSectionElement(parent, section, nextSection); table.onSectionRendered(section); + reattachSectionElement(parent, section, nextSection); // Detach the event listener. if (!table.isAttached()) { @@ -597,8 +597,8 @@ public final void replaceChildren(AbstractCellTable table, TableSectionElemen * Reattach the section. If next section is null, the section will be * appended instead. */ - reattachSectionElement(parent, section, nextSection); table.onSectionRendered(section); + reattachSectionElement(parent, section, nextSection); // Detach the event listener. if (!table.isAttached()) { @@ -1709,8 +1709,9 @@ protected boolean dependsOnSelection() { protected abstract void doSetHeaderVisible(boolean isFooter, boolean isVisible); /** - * Called after the rows of a head, body or foot section are replaced, so - * subclasses can post-process the rendered rows. The default does nothing. + * Called after the rows of a head, body or foot section are replaced, while + * the section is still detached from the document, so subclasses can + * post-process the rendered rows. The default does nothing. * * @param section the section whose rows were just rendered */ From 0692a4d28ac1a99d2ee983a927b1ba7435e97ded Mon Sep 17 00:00:00 2001 From: Gary Ritchie Date: Sun, 18 Oct 2026 12:34:48 -0600 Subject: [PATCH 3/8] Test DataGrid row headers and message containers through the DOM Cover the rowheader role for a th in the body, and check the loading and empty-table containers by putting the grid in those states and reading the table that holds the displayed widget. The containers go back to private, since the test no longer reaches into them. Addresses rstudio/rstudio#19173 --- .../gwt/user/cellview/client/DataGrid.java | 4 +- .../user/cellview/client/DataGridTest.java | 68 ++++++++++++++++--- 2 files changed, 61 insertions(+), 11 deletions(-) diff --git a/user/src/com/google/gwt/user/cellview/client/DataGrid.java b/user/src/com/google/gwt/user/cellview/client/DataGrid.java index 8f6dca4e9ed..1f925ccd6dc 100644 --- a/user/src/com/google/gwt/user/cellview/client/DataGrid.java +++ b/user/src/com/google/gwt/user/cellview/client/DataGrid.java @@ -538,9 +538,9 @@ private static void applyTableRoles(TableSectionElement section) { final TableWidget tableData; final TableWidget tableFooter; final TableWidget tableHeader; - final FlexTable emptyTableWidgetContainer; + private final FlexTable emptyTableWidgetContainer; private final HeaderPanel headerPanel; - final FlexTable loadingIndicatorContainer; + private final FlexTable loadingIndicatorContainer; private final Style style; private final Element tableDataContainer; private final ScrollPanel tableDataScroller; diff --git a/user/test/com/google/gwt/user/cellview/client/DataGridTest.java b/user/test/com/google/gwt/user/cellview/client/DataGridTest.java index a778548e065..194d3a8fdbc 100644 --- a/user/test/com/google/gwt/user/cellview/client/DataGridTest.java +++ b/user/test/com/google/gwt/user/cellview/client/DataGridTest.java @@ -18,6 +18,7 @@ import com.google.gwt.cell.client.TextCell; import com.google.gwt.core.client.GWT; import com.google.gwt.dom.builder.shared.TableRowBuilder; +import com.google.gwt.dom.client.Element; import com.google.gwt.dom.client.TableCellElement; import com.google.gwt.dom.client.TableElement; import com.google.gwt.dom.client.TableRowElement; @@ -25,7 +26,9 @@ import com.google.gwt.user.cellview.client.DataGrid.Resources; import com.google.gwt.user.client.ui.Label; import com.google.gwt.user.client.ui.RootPanel; +import com.google.gwt.user.client.ui.Widget; +import java.util.ArrayList; import java.util.Arrays; /** @@ -184,16 +187,51 @@ public void testTableRolesKeepAriaSort() { } /** - * Test that the empty-table and loading containers do not read as tables. + * Test that a th in the body is a row header. */ - public void testEmptyAndLoadingContainersArePresentational() { - DataGrid table = createAbstractHasData(); - table.setEmptyTableWidget(new Label("Empty")); - table.setLoadingIndicator(new Label("Loading")); - assertEquals("presentation", - table.emptyTableWidgetContainer.getElement().getAttribute("role")); - assertEquals("presentation", - table.loadingIndicatorContainer.getElement().getAttribute("role")); + public void testTableRolesBodyThIsRowHeader() { + DataGrid table = createRoleTestGrid(); + + table.setTableBuilder(new DefaultCellTableBuilder(table) { + @Override + public void buildRowImpl(String rowValue, int absRowIndex) { + TableRowBuilder tr = startRow(rowValue); + tr.startTH().text(rowValue).endTH(); + tr.startTD().text(rowValue).endTD(); + tr.endTR(); + } + }); + table.getPresenter().flush(); + TableRowElement row = table.getRowElement(0); + assertEquals("rowheader", row.getCells().getItem(0).getAttribute("role")); + assertEquals("cell", row.getCells().getItem(1).getAttribute("role")); + + RootPanel.get().remove(table); + } + + /** + * Test that the loading and empty-table messages, which replace the body + * table, are not shown inside a table of their own. + */ + public void testLoadingAndEmptyMessagesArePresentational() { + DataGrid table = createAbstractHasData(new TextCell()); + Label loading = new Label("Loading"); + Label empty = new Label("Empty"); + table.setLoadingIndicator(loading); + table.setEmptyTableWidget(empty); + + // A grid with no data yet is loading. + RootPanel.get().add(table); + table.getPresenter().flush(); + assertTrue(loading.isAttached()); + assertEquals("presentation", getEnclosingTable(table, loading).getAttribute("role")); + + table.setRowData(new ArrayList()); + table.getPresenter().flush(); + assertTrue(empty.isAttached()); + assertEquals("presentation", getEnclosingTable(table, empty).getAttribute("role")); + + RootPanel.get().remove(table); } @Override @@ -206,6 +244,18 @@ protected String getHeaderCellRole() { return "columnheader"; } + /** + * Get the nearest table element around a widget shown inside the grid. + */ + private static Element getEnclosingTable(DataGrid table, Widget widget) { + Element elem = widget.getElement(); + while (!"table".equalsIgnoreCase(elem.getTagName())) { + elem = elem.getParentElement(); + } + assertTrue(table.getElement().isOrHasChild(elem)); + return elem; + } + /** * Create an attached grid with three columns, the last one sortable, and * three rows. From 1a73a40470590b4a1e3e717fe25a6a053a36c409 Mon Sep 17 00:00:00 2001 From: Gary Ritchie Date: Sun, 18 Oct 2026 12:56:17 -0600 Subject: [PATCH 4/8] Stamp roles only on the rows a partial DataGrid redraw inserts replaceChildren passed the whole tbody to onSectionRendered, so updating one row re-checked the role of every row and cell in the body. It now passes the temporary section holding only the new rows, before they are moved into the table. The call in replaceAllRows moves above the reattach comment, which it had split from the line it describes. Addresses rstudio/rstudio#19173 --- .../user/cellview/client/AbstractCellTable.java | 14 ++++++++------ .../gwt/user/cellview/client/DataGridTest.java | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java b/user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java index 998dce98b3b..dc4434a5c85 100644 --- a/user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java +++ b/user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java @@ -523,12 +523,12 @@ public final void replaceAllRows(AbstractCellTable table, TableSectionElement // Render the html. replaceAllRowsImpl(table, section, html); + table.onSectionRendered(section); /* * Reattach the section. If next section is null, the section will be * appended instead. */ - table.onSectionRendered(section); reattachSectionElement(parent, section, nextSection); // Detach the event listener. @@ -586,6 +586,7 @@ public final void replaceChildren(AbstractCellTable table, TableSectionElemen // Add new child elements. TableSectionElement newSection = convertToSectionElement(table, section.getTagName(), html); + table.onSectionRendered(newSection); Element newChild = newSection.getFirstChildElement(); while (newChild != null) { Element next = newChild.getNextSiblingElement(); @@ -597,7 +598,6 @@ public final void replaceChildren(AbstractCellTable table, TableSectionElemen * Reattach the section. If next section is null, the section will be * appended instead. */ - table.onSectionRendered(section); reattachSectionElement(parent, section, nextSection); // Detach the event listener. @@ -1709,11 +1709,13 @@ protected boolean dependsOnSelection() { protected abstract void doSetHeaderVisible(boolean isFooter, boolean isVisible); /** - * Called after the rows of a head, body or foot section are replaced, while - * the section is still detached from the document, so subclasses can - * post-process the rendered rows. The default does nothing. + * Called after rows of a head, body or foot section are rendered, before + * they enter the document, so subclasses can post-process them. The default + * does nothing. * - * @param section the section whose rows were just rendered + * @param section after a full render, the table's own section, detached; + * after a partial render of the body, a detached section of the + * same tag holding only the new rows */ protected void onSectionRendered(TableSectionElement section) { } diff --git a/user/test/com/google/gwt/user/cellview/client/DataGridTest.java b/user/test/com/google/gwt/user/cellview/client/DataGridTest.java index 194d3a8fdbc..728f0cc4c2a 100644 --- a/user/test/com/google/gwt/user/cellview/client/DataGridTest.java +++ b/user/test/com/google/gwt/user/cellview/client/DataGridTest.java @@ -143,6 +143,22 @@ public void testTableRolesAfterPartialRedraw() { RootPanel.get().remove(table); } + /** + * Test that a partial redraw only stamps the rows it inserts, so updating + * one row doesn't walk the whole body. + */ + public void testPartialRedrawOnlyStampsNewRows() { + DataGrid table = createRoleTestGrid(); + + table.getRowElement(0).removeAttribute("role"); + table.setRowData(1, Arrays.asList("B")); + table.getPresenter().flush(); + assertFalse(table.getRowElement(0).hasAttribute("role")); + assertEquals("row", table.getRowElement(1).getAttribute("role")); + + RootPanel.get().remove(table); + } + /** * Test that a row the builder marks presentational keeps that role and its * cells get none. From 05c0d87c16cc69d2fbcaee80858472d0db0d8a1c Mon Sep 17 00:00:00 2001 From: Gary Ritchie Date: Sun, 18 Oct 2026 12:56:18 -0600 Subject: [PATCH 5/8] Treat an empty role as no role when stamping DataGrid rows A row with role="" was skipped along with its cells, and a cell with role="" kept the empty role, dropping them from the table. Each role is now read once and an empty one counts as absent. The th role is chosen once per section instead of inside the cell loop. Addresses rstudio/rstudio#19173 --- .../gwt/user/cellview/client/DataGrid.java | 18 ++++++++------- .../user/cellview/client/DataGridTest.java | 23 +++++++++++++++++++ 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/user/src/com/google/gwt/user/cellview/client/DataGrid.java b/user/src/com/google/gwt/user/cellview/client/DataGrid.java index 1f925ccd6dc..68135b44798 100644 --- a/user/src/com/google/gwt/user/cellview/client/DataGrid.java +++ b/user/src/com/google/gwt/user/cellview/client/DataGrid.java @@ -511,26 +511,28 @@ private static Resources getDefaultResources() { * Give the rows and cells of a rendered section explicit ARIA roles, which * they lose to the presentation role on their table. Rows and cells that * already have a role keep it, and a row with a role other than row (such - * as a layout row marked presentation) is left alone, cells included. + * as a layout row marked presentation) is left alone, cells included. An + * empty role counts as none. */ private static void applyTableRoles(TableSectionElement section) { - boolean isBody = "tbody".equalsIgnoreCase(section.getTagName()); + // Tag, not identity: a partial render passes a temporary tbody. + String thRole = "tbody".equalsIgnoreCase(section.getTagName()) ? "rowheader" : "columnheader"; NodeList rows = section.getRows(); for (int r = 0; r < rows.getLength(); r++) { TableRowElement row = rows.getItem(r); - if (!row.hasAttribute("role")) { + String rowRole = row.getAttribute("role"); + if (rowRole.isEmpty()) { row.setAttribute("role", "row"); - } else if (!"row".equals(row.getAttribute("role"))) { + } else if (!"row".equals(rowRole)) { continue; } NodeList cells = row.getCells(); for (int c = 0; c < cells.getLength(); c++) { TableCellElement cell = cells.getItem(c); - if (cell.hasAttribute("role")) { - continue; + if (cell.getAttribute("role").isEmpty()) { + boolean isTh = "th".equalsIgnoreCase(cell.getTagName()); + cell.setAttribute("role", isTh ? thRole : "cell"); } - boolean isTh = "th".equalsIgnoreCase(cell.getTagName()); - cell.setAttribute("role", isTh ? (isBody ? "rowheader" : "columnheader") : "cell"); } } } diff --git a/user/test/com/google/gwt/user/cellview/client/DataGridTest.java b/user/test/com/google/gwt/user/cellview/client/DataGridTest.java index 728f0cc4c2a..c82a3879921 100644 --- a/user/test/com/google/gwt/user/cellview/client/DataGridTest.java +++ b/user/test/com/google/gwt/user/cellview/client/DataGridTest.java @@ -159,6 +159,29 @@ public void testPartialRedrawOnlyStampsNewRows() { RootPanel.get().remove(table); } + /** + * Test that an empty role attribute counts as no role. + */ + public void testTableRolesTreatEmptyRoleAsAbsent() { + DataGrid table = createRoleTestGrid(); + + table.setTableBuilder(new DefaultCellTableBuilder(table) { + @Override + public void buildRowImpl(String rowValue, int absRowIndex) { + TableRowBuilder tr = startRow(rowValue); + tr.attribute("role", ""); + tr.startTD().attribute("role", "").text(rowValue).endTD(); + tr.endTR(); + } + }); + table.getPresenter().flush(); + TableRowElement row = table.getRowElement(0); + assertEquals("row", row.getAttribute("role")); + assertEquals("cell", row.getCells().getItem(0).getAttribute("role")); + + RootPanel.get().remove(table); + } + /** * Test that a row the builder marks presentational keeps that role and its * cells get none. From aaaf13382c0f3c4fbef707c2d8a8ad40c1c24753 Mon Sep 17 00:00:00 2001 From: Gary Ritchie Date: Sun, 18 Oct 2026 12:56:19 -0600 Subject: [PATCH 6/8] Expose DataGrid loading and empty messages as a table row The loading and empty-table containers are presentational, so their message sat inside the grid's table but outside any row. Chrome exposed it as bare text, which screen readers that navigate Chrome's table structure have no row to reach. The container's tbody, row and cell now get rowgroup, row and cell roles, so the message reads as a row of the grid. Addresses rstudio/rstudio#19173 --- .../gwt/user/cellview/client/DataGrid.java | 16 +++++++++-- .../user/cellview/client/DataGridTest.java | 28 +++++++++++-------- 2 files changed, 31 insertions(+), 13 deletions(-) diff --git a/user/src/com/google/gwt/user/cellview/client/DataGrid.java b/user/src/com/google/gwt/user/cellview/client/DataGrid.java index 68135b44798..f1cf00715d5 100644 --- a/user/src/com/google/gwt/user/cellview/client/DataGrid.java +++ b/user/src/com/google/gwt/user/cellview/client/DataGrid.java @@ -537,6 +537,18 @@ private static void applyTableRoles(TableSectionElement section) { } } + /** + * Show a widget in a message container (loading or empty table), whose + * single cell reads as a row of the grid. + */ + private static void setMessageWidget(FlexTable container, Widget widget) { + container.setWidget(0, 0, widget); + TableElement table = container.getElement().cast(); + table.getTBodies().getItem(0).setAttribute("role", "rowgroup"); + container.getRowFormatter().getElement(0).setAttribute("role", "row"); + container.getCellFormatter().getElement(0, 0).setAttribute("role", "cell"); + } + final TableWidget tableData; final TableWidget tableFooter; final TableWidget tableHeader; @@ -771,13 +783,13 @@ public void setAlwaysShowScrollBars(boolean alwaysShowScrollBars) { @Override public void setEmptyTableWidget(Widget widget) { - emptyTableWidgetContainer.setWidget(0, 0, widget); + setMessageWidget(emptyTableWidgetContainer, widget); super.setEmptyTableWidget(widget); } @Override public void setLoadingIndicator(Widget widget) { - loadingIndicatorContainer.setWidget(0, 0, widget); + setMessageWidget(loadingIndicatorContainer, widget); super.setLoadingIndicator(widget); } diff --git a/user/test/com/google/gwt/user/cellview/client/DataGridTest.java b/user/test/com/google/gwt/user/cellview/client/DataGridTest.java index c82a3879921..fbecd6e17db 100644 --- a/user/test/com/google/gwt/user/cellview/client/DataGridTest.java +++ b/user/test/com/google/gwt/user/cellview/client/DataGridTest.java @@ -30,6 +30,7 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Locale; /** * Tests for {@link DataGrid}. @@ -250,7 +251,7 @@ public void buildRowImpl(String rowValue, int absRowIndex) { /** * Test that the loading and empty-table messages, which replace the body - * table, are not shown inside a table of their own. + * table, read as a row of the grid rather than a table of their own. */ public void testLoadingAndEmptyMessagesArePresentational() { DataGrid table = createAbstractHasData(new TextCell()); @@ -263,12 +264,12 @@ public void testLoadingAndEmptyMessagesArePresentational() { RootPanel.get().add(table); table.getPresenter().flush(); assertTrue(loading.isAttached()); - assertEquals("presentation", getEnclosingTable(table, loading).getAttribute("role")); + assertMessageRoles(table, loading); table.setRowData(new ArrayList()); table.getPresenter().flush(); assertTrue(empty.isAttached()); - assertEquals("presentation", getEnclosingTable(table, empty).getAttribute("role")); + assertMessageRoles(table, empty); RootPanel.get().remove(table); } @@ -284,15 +285,20 @@ protected String getHeaderCellRole() { } /** - * Get the nearest table element around a widget shown inside the grid. + * Assert that a message widget shown in the grid sits in a cell of a row of + * a presentational table inside the grid. */ - private static Element getEnclosingTable(DataGrid table, Widget widget) { - Element elem = widget.getElement(); - while (!"table".equalsIgnoreCase(elem.getTagName())) { - elem = elem.getParentElement(); - } - assertTrue(table.getElement().isOrHasChild(elem)); - return elem; + private static void assertMessageRoles(DataGrid table, Widget message) { + Element td = message.getElement().getParentElement(); + Element tr = td.getParentElement(); + Element tbody = tr.getParentElement(); + Element tableElem = tbody.getParentElement(); + assertEquals("td", td.getTagName().toLowerCase(Locale.ROOT)); + assertTrue(table.getElement().isOrHasChild(tableElem)); + assertEquals("cell", td.getAttribute("role")); + assertEquals("row", tr.getAttribute("role")); + assertEquals("rowgroup", tbody.getAttribute("role")); + assertEquals("presentation", tableElem.getAttribute("role")); } /** From 125dca0188eb19f2ca066d98dd31a946ca99e7df Mon Sep 17 00:00:00 2001 From: Gary Ritchie Date: Sun, 18 Oct 2026 12:56:19 -0600 Subject: [PATCH 7/8] Test DataGrid footer roles and require no role on CellTable headers Cover the footer row and th roles. The sortable-header test compared the CellTable header's role to "", which also passed for role=""; it now requires the attribute to be absent. Addresses rstudio/rstudio#19173 --- .../client/AbstractCellTableTestBase.java | 19 +++++++++++---- .../user/cellview/client/DataGridTest.java | 24 +++++++++++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/user/test/com/google/gwt/user/cellview/client/AbstractCellTableTestBase.java b/user/test/com/google/gwt/user/cellview/client/AbstractCellTableTestBase.java index c3479a1efb1..1ba893a8e0c 100644 --- a/user/test/com/google/gwt/user/cellview/client/AbstractCellTableTestBase.java +++ b/user/test/com/google/gwt/user/cellview/client/AbstractCellTableTestBase.java @@ -1224,7 +1224,7 @@ public void testSortableHeaderAria() { // The sortable th keeps its columnheader role; the button is inside it. TableCellElement sortableTh = getHeaderElement(table, 0); - assertEquals(getHeaderCellRole(), sortableTh.getAttribute("role")); + assertHeaderCellRole(sortableTh); assertFalse(sortableTh.hasAttribute("aria-sort")); Element sortButton = sortableTh.getFirstChildElement(); assertEquals("button", sortButton.getAttribute("role")); @@ -1233,7 +1233,7 @@ public void testSortableHeaderAria() { // A column that isn't sortable gets no button. TableCellElement plainTh = getHeaderElement(table, 1); - assertEquals(getHeaderCellRole(), plainTh.getAttribute("role")); + assertHeaderCellRole(plainTh); assertFalse(plainTh.hasAttribute("aria-sort")); assertNull(plainTh.getFirstChildElement()); assertEquals("Column 1", plainTh.getInnerText()); @@ -1375,11 +1375,20 @@ public String getValue(String object) { } /** - * Get the role attribute a header th should carry: empty for an implicit - * columnheader, or the explicit role the table stamps on it. + * Get the role attribute a header th should carry: null for none (an + * implicit columnheader), or the explicit role the table stamps on it. */ protected String getHeaderCellRole() { - return ""; + return null; + } + + private void assertHeaderCellRole(TableCellElement th) { + String role = getHeaderCellRole(); + if (role == null) { + assertFalse(th.hasAttribute("role")); + } else { + assertEquals(role, th.getAttribute("role")); + } } /** diff --git a/user/test/com/google/gwt/user/cellview/client/DataGridTest.java b/user/test/com/google/gwt/user/cellview/client/DataGridTest.java index fbecd6e17db..90b44a187b9 100644 --- a/user/test/com/google/gwt/user/cellview/client/DataGridTest.java +++ b/user/test/com/google/gwt/user/cellview/client/DataGridTest.java @@ -160,6 +160,30 @@ public void testPartialRedrawOnlyStampsNewRows() { RootPanel.get().remove(table); } + /** + * Test that footer rows and their th cells get roles, like the header. + */ + public void testTableRolesFooter() { + DataGrid table = createAbstractHasData(); + table.addColumn(new TextColumn() { + @Override + public String getValue(String object) { + return object; + } + }, new TextHeader("Name"), new TextHeader("Total")); + RootPanel.get().add(table); + table.setRowData(Arrays.asList("a")); + table.getPresenter().flush(); + + TableRowElement footerRow = table.getTableFootElement().getRows().getItem(0); + assertEquals("row", footerRow.getAttribute("role")); + TableCellElement footerCell = footerRow.getCells().getItem(0); + assertEquals("Total", footerCell.getInnerText()); + assertEquals("columnheader", footerCell.getAttribute("role")); + + RootPanel.get().remove(table); + } + /** * Test that an empty role attribute counts as no role. */ From ea4d4231574131b29e54f740636f7eb2d28b923f Mon Sep 17 00:00:00 2001 From: Gary Ritchie Date: Sun, 18 Oct 2026 13:05:37 -0600 Subject: [PATCH 8/8] Leave a disabled DataGrid message out of the table Passing null to setLoadingIndicator or setEmptyTableWidget disables the message, but its container still has an empty cell, which the row and cell roles exposed as a blank row of the grid. The roles are now only set while the container holds a widget. Addresses rstudio/rstudio#19173 --- .../gwt/user/cellview/client/DataGrid.java | 18 ++++++++++++---- .../user/cellview/client/DataGridTest.java | 21 +++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/user/src/com/google/gwt/user/cellview/client/DataGrid.java b/user/src/com/google/gwt/user/cellview/client/DataGrid.java index f1cf00715d5..f43b867ae28 100644 --- a/user/src/com/google/gwt/user/cellview/client/DataGrid.java +++ b/user/src/com/google/gwt/user/cellview/client/DataGrid.java @@ -539,14 +539,24 @@ private static void applyTableRoles(TableSectionElement section) { /** * Show a widget in a message container (loading or empty table), whose - * single cell reads as a row of the grid. + * single cell reads as a row of the grid. A null widget disables the + * message, so its empty cell must not read as a blank row. */ private static void setMessageWidget(FlexTable container, Widget widget) { container.setWidget(0, 0, widget); TableElement table = container.getElement().cast(); - table.getTBodies().getItem(0).setAttribute("role", "rowgroup"); - container.getRowFormatter().getElement(0).setAttribute("role", "row"); - container.getCellFormatter().getElement(0, 0).setAttribute("role", "cell"); + Element[] parts = { + table.getTBodies().getItem(0), + container.getRowFormatter().getElement(0), + container.getCellFormatter().getElement(0, 0)}; + String[] roles = {"rowgroup", "row", "cell"}; + for (int i = 0; i < parts.length; i++) { + if (widget == null) { + parts[i].removeAttribute("role"); + } else { + parts[i].setAttribute("role", roles[i]); + } + } } final TableWidget tableData; diff --git a/user/test/com/google/gwt/user/cellview/client/DataGridTest.java b/user/test/com/google/gwt/user/cellview/client/DataGridTest.java index 90b44a187b9..c0a2f19a507 100644 --- a/user/test/com/google/gwt/user/cellview/client/DataGridTest.java +++ b/user/test/com/google/gwt/user/cellview/client/DataGridTest.java @@ -19,6 +19,7 @@ import com.google.gwt.core.client.GWT; import com.google.gwt.dom.builder.shared.TableRowBuilder; import com.google.gwt.dom.client.Element; +import com.google.gwt.dom.client.NodeList; import com.google.gwt.dom.client.TableCellElement; import com.google.gwt.dom.client.TableElement; import com.google.gwt.dom.client.TableRowElement; @@ -250,6 +251,26 @@ public void testTableRolesKeepAriaSort() { RootPanel.get().remove(table); } + /** + * Test that a disabled (null) loading indicator leaves no blank row in the + * grid while it loads. + */ + public void testNullLoadingIndicatorAddsNoRow() { + DataGrid table = createAbstractHasData(new TextCell()); + table.setLoadingIndicator(new Label("Loading")); + table.setLoadingIndicator(null); + RootPanel.get().add(table); + table.getPresenter().flush(); + + // While loading, the only td in the grid is the indicator container's. + NodeList tds = table.getElement().getElementsByTagName("td"); + assertEquals(1, tds.getLength()); + assertFalse(tds.getItem(0).hasAttribute("role")); + assertFalse(tds.getItem(0).getParentElement().hasAttribute("role")); + + RootPanel.get().remove(table); + } + /** * Test that a th in the body is a row header. */