Support ARIA table attributes, skip layout tables, and don't get stuck on hidden cells

nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp

@@ -78,13 +78,18 @@ IAccessible2* IAccessible2FromIdentifier(int docHandle, int ID) {
 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 two sets of attributes on this vbuf node.
+	// The first set: table-physicalrowcount and table-physicalcolumncount represent the physical topology of the table and can be used programmatically to understand table limits.
+	// The second set: table-rowcount and table-columncount are duplicates of the physical ones, however may be overridden later on in fillVBuf with ARIA attributes. They are what is reported to the user.
 	if (paccTable->get_nRows(&count) == S_OK) {
 		s << count;
+		node->addAttribute(L"table-physicalrowcount", s.str());
 		node->addAttribute(L"table-rowcount", s.str());
 		s.str(L"");
 	}
 	if (paccTable->get_nColumns(&count) == S_OK) {
 		s << count;
+		node->addAttribute(L"table-physicalcolumncount", s.str());
 		node->addAttribute(L"table-columncount", s.str());
 	}
 }
@@ -128,11 +133,17 @@ inline void fillTableCellInfo_IATable(VBufStorage_controlFieldNode_t* node, IAcc
 	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.
+	// for rowNumber and columnNumber, store these as two sets of attributes.
+	// The first set: table-physicalrownumber and table-physicalcolumnnumber represent the physical topology of the table and can be used programmatically to fetch other table cells with IAccessibleTable etc.
+	// The second set: table-rownumber and table-columnnumber are duplicates of the physical ones, however may be overridden later on in fillVBuf with ARIA attributes. They are what is reported to the user.
 	if (paccTable->get_rowColumnExtentsAtIndex(cellIndex, &row, &column, &rowExtents, &columnExtents, &isSelected) == S_OK) {
 		s << row + 1;
+		node->addAttribute(L"table-physicalrownumber", s.str());
 		node->addAttribute(L"table-rownumber", s.str());
 		s.str(L"");
 		s << column + 1;
+		node->addAttribute(L"table-physicalcolumnnumber", s.str());
 		node->addAttribute(L"table-columnnumber", s.str());
 		if (columnExtents > 1) {
 			s.str(L"");
@@ -186,11 +197,17 @@ inline void GeckoVBufBackend_t::fillTableCellInfo_IATable2(VBufStorage_controlFi
 
 	long row, column, rowExtents, columnExtents;
 	boolean isSelected;
+	// Fetch row and column extents and add them as attributes on this node.
+	// for rowNumber and columnNumber, store these as two sets of attributes.
+	// The first set: table-physicalrownumber and table-physicalcolumnnumber represent the physical topology of the table and can be used programmatically to fetch other table cells with IAccessibleTable etc.
+	// The second set: table-rownumber and table-columnnumber are duplicates of the physical ones, however may be overridden later on in fillVBuf with ARIA attributes. They are what is reported to the user.
 	if (paccTableCell->get_rowColumnExtents(&row, &column, &rowExtents, &columnExtents, &isSelected) == S_OK) {
 		s << row + 1;
+		node->addAttribute(L"table-physicalrownumber", s.str());
 		node->addAttribute(L"table-rownumber", s.str());
 		s.str(L"");
 		s << column + 1;
+		node->addAttribute(L"table-physicalcolumnnumber", s.str());
 		node->addAttribute(L"table-columnnumber", s.str());
 		if (columnExtents > 1) {
 			s.str(L"");
@@ -305,7 +322,7 @@ const wregex REGEX_PRESENTATION_ROLE(L"IAccessible2\\\\:\\\\:attribute_xml-roles
 
 VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(IAccessible2* pacc,
 	VBufStorage_buffer_t* buffer, VBufStorage_controlFieldNode_t* parentNode, VBufStorage_fieldNode_t* previousNode,
-	IAccessibleTable* paccTable, IAccessibleTable2* paccTable2, long tableID,
+	IAccessibleTable* paccTable, IAccessibleTable2* paccTable2, long tableID, const wchar_t* parentPresentationalRowNumber,
 	bool ignoreInteractiveUnlabelledGraphics
 ) {
 	nhAssert(buffer); //buffer can't be NULL
@@ -655,6 +672,23 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(IAccessible2* pacc,
 		}
 	}
 
+	// Add some presentational table attributes
+	// Note these are only for reporting, the physical table attributes (table-physicalrownumber etc) for aiding in navigation etc are added  later on.
+	// propagate table-rownumber down to the cell as Gecko only includes it on the row itself
+	if(parentPresentationalRowNumber) 
+		parentNode->addAttribute(L"table-rownumber",parentPresentationalRowNumber);
+	const wchar_t* presentationalRowNumber=NULL;
+	if((IA2AttribsMapIt = IA2AttribsMap.find(L"rowindex")) != IA2AttribsMap.end()) {
+		parentNode->addAttribute(L"table-rownumber",IA2AttribsMapIt->second);
+		presentationalRowNumber=IA2AttribsMapIt->second.c_str();
+	}
+	if((IA2AttribsMapIt = IA2AttribsMap.find(L"colindex")) != IA2AttribsMap.end())
+		parentNode->addAttribute(L"table-columnnumber",IA2AttribsMapIt->second);
+	if((IA2AttribsMapIt = IA2AttribsMap.find(L"rowcount")) != IA2AttribsMap.end())
+		parentNode->addAttribute(L"table-rowcount",IA2AttribsMapIt->second);
+	if((IA2AttribsMapIt = IA2AttribsMap.find(L"colcount")) != IA2AttribsMap.end())
+		parentNode->addAttribute(L"table-columncount",IA2AttribsMapIt->second);
+
 	BSTR value=NULL;
 	if(pacc->get_accValue(varChild,&value)==S_OK) {
 		if(value&&SysStringLen(value)==0) {
@@ -736,7 +770,7 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(IAccessible2* pacc,
 						continue;
 					}
 					paccHyperlink->Release();
-					if (tempNode = this->fillVBuf(childPacc, buffer, parentNode, previousNode, paccTable, paccTable2, tableID, ignoreInteractiveUnlabelledGraphics)) {
+					if (tempNode = this->fillVBuf(childPacc, buffer, parentNode, previousNode, paccTable, paccTable2, tableID, presentationalRowNumber, ignoreInteractiveUnlabelledGraphics)) {
 						previousNode=tempNode;
 					} else {
 						LOG_DEBUG(L"Error in fillVBuf");
@@ -768,7 +802,7 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(IAccessible2* pacc,
 					VariantClear(&(varChildren[i]));
 					continue;
 				}
-				if (tempNode = this->fillVBuf(childPacc, buffer, parentNode, previousNode, paccTable, paccTable2, tableID, ignoreInteractiveUnlabelledGraphics))
+				if (tempNode = this->fillVBuf(childPacc, buffer, parentNode, previousNode, paccTable, paccTable2, tableID, presentationalRowNumber, ignoreInteractiveUnlabelledGraphics))
 					previousNode=tempNode;
 				else
 					LOG_DEBUG(L"Error in calling fillVBuf");

nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.h

@@ -22,7 +22,7 @@ class GeckoVBufBackend_t: public VBufBackend_t {
 
 	VBufStorage_fieldNode_t* fillVBuf(IAccessible2* pacc,
 		VBufStorage_buffer_t* buffer, VBufStorage_controlFieldNode_t* parentNode, VBufStorage_fieldNode_t* previousNode,
-		IAccessibleTable* paccTable=NULL, IAccessibleTable2* paccTable2=NULL, long tableID=0,
+		IAccessibleTable* paccTable=NULL, IAccessibleTable2* paccTable2=NULL, long tableID=0, const wchar_t* parentPresentationalRowNumber=NULL,
 		bool ignoreInteractiveUnlabelledGraphics=false
 	);
 

source/NVDAObjects/IAccessible/__init__.py

@@ -1022,6 +1022,8 @@ def getChild(self, index):
 		return self.correctAPIForRelation(IAccessible(IAccessibleObject=child[0], IAccessibleChildID=child[1]))
 
 	def _get_IA2Attributes(self):
+		if not isinstance(self.IAccessibleObject,IAccessibleHandler.IAccessible2):
+			return {}
 		try:
 			attribs = self.IAccessibleObject.attributes
 		except COMError as e:
@@ -1035,7 +1037,7 @@ def event_IA2AttributeChange(self):
 		# We currently only care about changes to the accessible drag and drop attributes, which we map to states, so treat this as a stateChange.
 		self.event_stateChange()
 
-	def _get_rowNumber(self):
+	def _get_IA2PhysicalRowNumber(self):
 		table=self.table
 		if table:
 			if self.IAccessibleTableUsesTableCellIndexAttrib:
@@ -1052,7 +1054,15 @@ def _get_rowNumber(self):
 				log.debugWarning("IAccessibleTable::rowIndex failed", exc_info=True)
 		raise NotImplementedError
 
-	def _get_columnNumber(self):
+	def _get_rowNumber(self):
+		index=self.IA2Attributes.get('rowindex')
+		if index is None and isinstance(self.parent,IAccessible):
+			index=self.parent.IA2Attributes.get('rowindex')
+		if index is None:
+			index=self.IA2PhysicalRowNumber
+		return index
+
+	def _get_IA2PhysicalColumnNumber(self):
 		table=self.table
 		if table:
 			if self.IAccessibleTableUsesTableCellIndexAttrib:
@@ -1069,22 +1079,40 @@ def _get_columnNumber(self):
 				log.debugWarning("IAccessibleTable::columnIndex failed", exc_info=True)
 		raise NotImplementedError
 
-	def _get_rowCount(self):
+	def _get_columnNumber(self):
+		index=self.IA2Attributes.get('colindex')
+		if index is None:
+			index=self.IA2PhysicalColumnNumber
+		return index
+
+	def _get_IA2PhysicalRowCount(self):
 		if hasattr(self,'IAccessibleTableObject'):
 			try:
 				return self.IAccessibleTableObject.nRows
 			except COMError:
 				log.debugWarning("IAccessibleTable::nRows failed", exc_info=True)
 		raise NotImplementedError
 
-	def _get_columnCount(self):
+	def _get_rowCount(self):
+		count=self.IA2Attributes.get('rowcount')
+		if count is None:
+			count=self.IA2PhysicalRowCount
+		return count
+
+	def _get_IA2PhysicalColumnCount(self):
 		if hasattr(self,'IAccessibleTableObject'):
 			try:
 				return self.IAccessibleTableObject.nColumns
 			except COMError:
 				log.debugWarning("IAccessibleTable::nColumns failed", exc_info=True)
 		raise NotImplementedError
 
+	def _get_columnCount(self):
+		count=self.IA2Attributes.get('colcount')
+		if count is None:
+			count=self.IA2PhysicalColumnCount
+		return count
+
 	def _get__IATableCell(self):
 		# Permanently cache the result.
 		try:

source/browseMode.py

@@ -1539,6 +1539,11 @@ def script_movePastEndOfContainer(self,gesture):
 	# Translators: Description for the Move past end of container command in browse mode. 
 	script_movePastEndOfContainer.__doc__=_("Moves past the end  of the container element, such as a list or table")
 
+	#: The controlField attribute name that should be used as the row number when navigating in a table. By default this is the same as the presentational attribute name
+	navigationalTableRowNumberAttributeName="table-rownumber"
+	#: The controlField attribute name that should be used as the column number when navigating in a table. By default this is the same as the presentational attribute name
+	navigationalTableColumnNumberAttributeName="table-columnnumber"
+
 	def _getTableCellCoords(self, info):
 		"""
 		Fetches information about the deepest table cell at the given position.
@@ -1551,17 +1556,28 @@ def _getTableCellCoords(self, info):
 		if info.isCollapsed:
 			info = info.copy()
 			info.expand(textInfos.UNIT_CHARACTER)
-		for field in reversed(info.getTextWithFields()):
+		fields=list(info.getTextWithFields())
+		# First record the ID of all layout tables so that we can skip them when searching for the deepest table
+		layoutIDs=set()
+		for field in fields:
+			if isinstance(field, textInfos.FieldCommand) and field.command == "controlStart" and field.field.get('table-layout'):
+				tableID=field.field.get('table-id')
+				if tableID is not None:
+					layoutIDs.add(tableID)
+		for field in reversed(fields):
 			if not (isinstance(field, textInfos.FieldCommand) and field.command == "controlStart"):
 				# Not a control field.
 				continue
 			attrs = field.field
-			if "table-id" in attrs and "table-rownumber" in attrs:
+			tableID=attrs.get('table-id')
+			if tableID is None or tableID in layoutIDs:
+				continue
+			if self.navigationalTableColumnNumberAttributeName in attrs and not attrs.get('table-layout'):
 				break
 		else:
 			raise LookupError("Not in a table cell")
 		return (attrs["table-id"],
-			attrs["table-rownumber"], attrs["table-columnnumber"],
+			attrs[self.navigationalTableRowNumberAttributeName], attrs[self.navigationalTableColumnNumberAttributeName],
 			attrs.get("table-rowsspanned", 1), attrs.get("table-columnsspanned", 1))
 
 	def _getTableCellAt(self,tableID,startPos,row,column):
@@ -1576,13 +1592,16 @@ def _getTableCellAt(self,tableID,startPos,row,column):
 		@type column: int
 		@returns: the table cell's position in the document
 		@rtype: L{textInfos.TextInfo}
+		@raises: LookupError if the cell does not exist 
 		"""
 		raise NotImplementedError
 
+	_missingTableCellSearchLimit=3 #: The number of missing  cells L{_getNearestTableCell} is allowed to skip over to locate the next available cell
 	def _getNearestTableCell(self, tableID, startPos, origRow, origCol, origRowSpan, origColSpan, movement, axis):
 		"""
 		Locates the nearest table cell relative to another table cell in a given direction, given its coordinates.
 		For example, this is used to move to the cell in the next column, previous row, etc.
+		This method will skip over missing table cells (where L{_getTableCellAt} raises LookupError), up to the number of times set by _missingTableCellSearchLimit set on this instance.
 		@param tableID: the ID of the table
 		@param startPos: the position in the document to start searching from.
 		@type startPos: L{textInfos.TextInfo}
@@ -1612,19 +1631,28 @@ def _getNearestTableCell(self, tableID, startPos, origRow, origCol, origRowSpan,
 		elif axis == "column":
 			destCol += origColSpan if movement == "next" else -1
 
-		if destCol < 1 or destRow<1:
-			# Optimisation: We're definitely at the edge of the column or row.
-			raise LookupError
-
-		return self._getTableCellAt(tableID,startPos,destRow,destCol)
+		# Try and fetch the cell at these coordinates, though  if a  cell is missing, try  several more times moving the coordinates on by one cell each time
+		limit=self._missingTableCellSearchLimit
+		while limit>0:
+			limit-=1
+			if destCol < 1 or destRow<1:
+				# Optimisation: We're definitely at the edge of the column or row.
+				raise LookupError
+			try:
+				return self._getTableCellAt(tableID,startPos,destRow,destCol)
+			except LookupError:
+				pass
+			if axis=="row":
+				destRow+=1 if movement=="next" else -1
+			else:
+				destCol+=1 if movement=="next" else -1
+		raise LookupError
 
 	def _tableMovementScriptHelper(self, movement="next", axis=None):
 		if isScriptWaiting():
 			return
 		formatConfig=config.conf["documentFormatting"].copy()
 		formatConfig["reportTables"]=True
-		#  For now, table movement includes layout tables even if reporting of layout tables is disabled.
-		formatConfig["includeLayoutTables"]=True
 		try:
 			tableID, origRow, origCol, origRowSpan, origColSpan = self._getTableCellCoords(self.selection)
 		except LookupError:

source/virtualBuffers/gecko_ia2.py

@@ -23,6 +23,11 @@
 class Gecko_ia2_TextInfo(VirtualBufferTextInfo):
 
 	def _normalizeControlField(self,attrs):
+		for attr in ("table-physicalrownumber","table-physicalcolumnnumber","table-physicalrowcount","table-physicalcolumncount"):
+			attrVal=attrs.get(attr)
+			if attrVal is not None:
+				attrs[attr]=int(attrVal)
+
 		current = attrs.get("IAccessible2::attribute_current")
 		if current is not None:
 			attrs['current']= current
@@ -260,12 +265,20 @@ def event_scrollingStart(self, obj, nextHandler):
 			return nextHandler()
 	event_scrollingStart.ignoreIsReady = True
 
+	# NVDA exposes IAccessible2 table interface row and column numbers as table-physicalrownumber and table-physicalcolumnnumber respectively.
+	# These should be used when navigating the physical table (I.e. these values should be provided to the table interfaces).
+	# The presentational table-columnnumber and table-rownumber attributes are normally duplicates of the physical ones, but are overridden  by the values of aria-rowindex and aria-colindex if present.
+	navigationalTableRowNumberAttributeName="table-physicalrownumber"
+	navigationalTableColumnNumberAttributeName="table-physicalcolumnnumber"
+
 	def _getTableCellAt(self,tableID,startPos,destRow,destCol):
 		docHandle = self.rootDocHandle
 		table = self.getNVDAObjectFromIdentifier(docHandle, tableID)
 		try:
 			cell = table.IAccessibleTableObject.accessibleAt(destRow - 1, destCol - 1).QueryInterface(IAccessible2)
 			cell = NVDAObjects.IAccessible.IAccessible(IAccessibleObject=cell, IAccessibleChildID=0)
+			if cell.IA2Attributes.get('hidden'):
+				raise LookupError("Found hidden cell") 
 			return self.makeTextInfo(cell)
 		except (COMError, RuntimeError):
 			raise LookupError