Merge 1212759 into 3f6bf0c

LeonarddeR · web-flow · commit da1193ed69a9 · 2021-05-04T03:54:52.000+10:00
diff --git a/nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp b/nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp
@@ -105,21 +105,6 @@ static IAccessible2* IAccessible2FromIdentifier(int docHandle, int ID) {
 	return pacc2;
 }
 
-template<typename TableType> inline void fillTableCounts(VBufStorage_controlFieldNode_t* node, IAccessible2* pacc, TableType* paccTable) {
-	wostringstream s;
-	long count = 0;
-	// Fetch row and column counts and add them as attributes on this vbuf node.
-	if (paccTable->get_nRows(&count) == S_OK) {
-		s << count;
-		node->addAttribute(L"table-rowcount", s.str());
-		s.str(L"");
-	}
-	if (paccTable->get_nColumns(&count) == S_OK) {
-		s << count;
-		node->addAttribute(L"table-columncount", s.str());
-	}
-}
-
 inline int getTableIDFromCell(IAccessibleTableCell* tableCell) {
 	IUnknown* unk = NULL;
 	if (tableCell->get_table(&unk) != S_OK || !unk)
@@ -136,31 +121,6 @@ inline int getTableIDFromCell(IAccessibleTableCell* tableCell) {
 	return id;
 }
 
-inline void fillTableCellInfo_IATable(VBufStorage_controlFieldNode_t* node, IAccessibleTable* paccTable, const wstring& cellIndexStr) {
-	wostringstream s;
-	long cellIndex = _wtoi(cellIndexStr.c_str());
-	long row, column, rowExtents, columnExtents;
-	boolean isSelected;
-	// Fetch row and column extents and add them as attributes on this node.
-	if (paccTable->get_rowColumnExtentsAtIndex(cellIndex, &row, &column, &rowExtents, &columnExtents, &isSelected) == S_OK) {
-		s << row + 1;
-		node->addAttribute(L"table-rownumber", s.str());
-		s.str(L"");
-		s << column + 1;
-		node->addAttribute(L"table-columnnumber", s.str());
-		if (columnExtents > 1) {
-			s.str(L"");
-			s << columnExtents;
-			node->addAttribute(L"table-columnsspanned", s.str());
-		}
-		if (rowExtents > 1) {
-			s.str(L"");
-			s << rowExtents;
-			node->addAttribute(L"table-rowsspanned", s.str());
-		}
-	}
-}
-
 typedef HRESULT(STDMETHODCALLTYPE IAccessibleTableCell::*IATableCellGetHeaderCellsFunc)(IUnknown***, long*);
 inline void fillTableHeaders(VBufStorage_controlFieldNode_t* node, IAccessibleTableCell* paccTableCell, const IATableCellGetHeaderCellsFunc getHeaderCells, const wstring& attribName) {
 	wostringstream s;
@@ -379,7 +339,6 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(
 	VBufStorage_buffer_t* buffer,
 	VBufStorage_controlFieldNode_t* parentNode,
 	VBufStorage_fieldNode_t* previousNode,
-	IAccessibleTable* paccTable,
 	IAccessibleTable2* paccTable2,
 	long tableID,
 	const wchar_t* parentPresentationalRowNumber,
@@ -505,7 +464,7 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(
 		LOG_DEBUG(L"pacc->get_states failed");
 		IA2States=0;
 	}
-	// Remove state_editible from tables as Gecko exposes it for ARIA grids which is not in the ARIA spec. 
+	// Remove state_editable from tables as Gecko exposes it for ARIA grids which is not in the ARIA spec. 
 	if(IA2States&IA2_STATE_EDITABLE&&role==ROLE_SYSTEM_TABLE) {
 			IA2States-=IA2_STATE_EDITABLE;
 	}
@@ -732,7 +691,7 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(
 	}
 
 	// Handle table cell information.
-	IAccessibleTableCell* paccTableCell = NULL;
+	IAccessibleTableCell* paccTableCell = nullptr;
 	if(pacc->QueryInterface(IID_IAccessibleTableCell, (void**)&paccTableCell)!=S_OK) {
 		paccTableCell=nullptr;
 	}
@@ -764,56 +723,58 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(
 		}
 	}
 
-	// For IAccessibleTable, we must always be passed the table interface by the caller.
 	// For IAccessibleTable2, we can always obtain the cell interface,
 	// which allows us to handle updates to table cells.
-	if (
-		 paccTableCell || // IAccessibleTable2
-		(paccTable && (IA2AttribsMapIt = IA2AttribsMap.find(L"table-cell-index")) != IA2AttribsMap.end()) // IAccessibleTable
-	) {
-		if (paccTableCell) {
-			// IAccessibleTable2
-			this->fillTableCellInfo_IATable2(parentNode, paccTableCell);
-			if (!paccTable2) {
-				// This is an update; we're not rendering the entire table.
-				tableID = getTableIDFromCell(paccTableCell);
-			}
-			paccTableCell->Release();
-			paccTableCell = NULL;
-		} else // IAccessibleTable
-			fillTableCellInfo_IATable(parentNode, paccTable, IA2AttribsMapIt->second);
+	if (paccTableCell) {
+		this->fillTableCellInfo_IATable2(parentNode, paccTableCell);
+		if (!paccTable2) {
+			// This is an update; we're not rendering the entire table.
+			tableID = getTableIDFromCell(paccTableCell);
+		}
+		paccTableCell->Release();
+		paccTableCell = nullptr;
 		// tableID is the IAccessible2::uniqueID for paccTable.
 		s << tableID;
 		parentNode->addAttribute(L"table-id", s.str());
 		s.str(L"");
 		// We're now within a cell, so descendant nodes shouldn't refer to this table anymore.
-		paccTable = NULL;
-		paccTable2 = NULL;
+		paccTable2 = nullptr;
 		tableID = 0;
 	}
 	// Handle table information.
 	// Don't release the table unless it was created in this call.
-	bool releaseTable = false;
-	// If paccTable is not NULL, we're within a table but not yet within a cell, so don't bother to query for table info.
-	if (!paccTable2 && !paccTable) {
+	IAccessibleTable2* curNodePaccTable2 = nullptr;
+	// If paccTable2 is not NULL, we're within a table but not yet within a cell, so don't bother to query for table info.
+	if (!paccTable2) {
 		// Try to get table information.
-		pacc->QueryInterface(IID_IAccessibleTable2,(void**)&paccTable2);
-		if(!paccTable2)
-			pacc->QueryInterface(IID_IAccessibleTable,(void**)&paccTable);
-		if (paccTable2||paccTable) {
-			// We did the QueryInterface for paccTable, so we must release it after all calls that use it are done.
-			releaseTable = true;
+		pacc->QueryInterface(IID_IAccessibleTable2,(void**)&curNodePaccTable2);
+		if (curNodePaccTable2) {
 			// This is a table, so add its information as attributes.
-			if((IA2AttribsMapIt = IA2AttribsMap.find(L"layout-guess")) != IA2AttribsMap.end())
+			if((IA2AttribsMapIt = IA2AttribsMap.find(L"layout-guess")) != IA2AttribsMap.end()) {
 				parentNode->addAttribute(L"table-layout",L"1");
+			}
 			tableID = ID;
 			s << ID;
 			parentNode->addAttribute(L"table-id", s.str());
 			s.str(L"");
-			if(paccTable2)
-				fillTableCounts<IAccessibleTable2>(parentNode, pacc, paccTable2);
-			else
-				fillTableCounts<IAccessibleTable>(parentNode, pacc, paccTable);
+			// Fetch row and column counts and add them as attributes on the vbuf node.
+			long rowCount = 0;
+			if (curNodePaccTable2->get_nRows(&rowCount) == S_OK) {
+				s << rowCount;
+				parentNode->addAttribute(L"table-rowcount", s.str());
+				s.str(L"");
+			}
+			long colCount = 0;
+			if (curNodePaccTable2->get_nColumns(&colCount) == S_OK) {
+				s << colCount;
+				parentNode->addAttribute(L"table-columncount", s.str());
+				s.str(L"");
+			}
+			if (rowCount > 0 || colCount > 0) {
+				// This table has rows and columns.
+				// Maintain curNodePaccTable2 for child rendering until any table cells are found.
+				paccTable2 = curNodePaccTable2;
+			}
 			// Add the table summary if one is present and the table is visible.
 			if (isVisible &&
 				(!description.empty() && (tempNode = buffer->addTextFieldNode(parentNode, previousNode, description))) ||
@@ -939,7 +900,6 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(
 						buffer,
 						parentNode,
 						previousNode,
-						paccTable,
 						paccTable2,
 						tableID,
 						presentationalRowNumber,
@@ -981,7 +941,6 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(
 					buffer,
 					parentNode,
 					previousNode,
-					paccTable,
 					paccTable2,
 					tableID,
 					presentationalRowNumber,
@@ -1005,7 +964,6 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(
 					buffer,
 					parentNode,
 					previousNode,
-					paccTable,
 					paccTable2,
 					tableID,
 					presentationalRowNumber,
@@ -1032,7 +990,6 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(
 					buffer,
 					parentNode,
 					previousNode,
-					paccTable,
 					paccTable2,
 					tableID,
 					presentationalRowNumber,
@@ -1133,13 +1090,10 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(
 		SysFreeString(IA2Text);
 	if(paccText)
 		paccText->Release();
-	if (releaseTable) {
-		if(paccTable2)
-			paccTable2->Release();
-		else
-			paccTable->Release();
+	if (curNodePaccTable2) {
+		curNodePaccTable2->Release();
+		curNodePaccTable2 = nullptr;
 	}
-
 	return parentNode;
 }
 
diff --git a/nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.h b/nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.h
@@ -28,7 +28,6 @@ class GeckoVBufBackend_t: public VBufBackend_t {
 		VBufStorage_buffer_t* buffer,
 		VBufStorage_controlFieldNode_t* parentNode,
 		VBufStorage_fieldNode_t* previousNode,
-		IAccessibleTable* paccTable=NULL,
 		IAccessibleTable2* paccTable2=NULL,
 		long tableID=0, const wchar_t* parentPresentationalRowNumber=NULL,
 		bool ignoreInteractiveUnlabelledGraphics=false
diff --git a/tests/system/robot/chromeTests.py b/tests/system/robot/chromeTests.py
@@ -1,5 +1,5 @@
 # A part of NonVisual Desktop Access (NVDA)
-# Copyright (C) 2020 NV Access Limited
+# Copyright (C) 2020-2021 NV Access Limited, Leonard de Ruijter
 # This file may be used under the terms of the GNU General Public License, version 2 or later.
 # For more details see: https://www.gnu.org/licenses/gpl-2.0.html
 
@@ -379,3 +379,44 @@ def test_i12147():
 		actualSpeech,
 		"target 0  heading  level 4"
 	)
+
+
+def test_tableInStyleDisplayTable():
+	"""
+	Chrome treats nodes with `style="display: table"` as tables.
+	When a HTML style table is positioned in such a node, NVDA was previously unable to announce
+	table row and column count as well as provide table navigation for the inner table.
+	"""
+	_chrome.prepareChrome(
+		"""
+			<p>Paragraph</p>
+			<div style="display:table">
+				<table>
+					<thead>
+						<tr>
+							<th>First heading</th>
+							<th>Second heading</th>
+						</tr>
+					</thead>
+					<tbody>
+						<tr>
+							<td>First content cell</td>
+							<td>Second content cell</td>
+						</tr>
+					</tbody>
+				</table>
+			</div>
+		"""
+	)
+	# Jump to the table
+	actualSpeech = _chrome.getSpeechAfterKey("t")
+	_asserts.strings_match(
+		actualSpeech,
+		"table  with 2 rows and 2 columns  row 1  First heading  column 1  First heading"
+	)
+	nextActualSpeech = _chrome.getSpeechAfterKey("control+alt+downArrow")
+	_asserts.strings_match(
+		nextActualSpeech,
+		"row 2  First content cell"
+	)
+
diff --git a/tests/system/robot/chromeTests.robot b/tests/system/robot/chromeTests.robot
@@ -54,3 +54,6 @@ ARIA checkbox
 i12147
 	[Documentation]	New focus target should be announced if the triggering element is removed when activated
 	test_i12147
+Table in style display: table
+	[Documentation]	Properly announce table row/column count and working table navigation for a HTML table in a div with style display: table
+	test_tableInStyleDisplayTable
