Treat ARIA treegrids as tables in browse mode (#11699)

* Chrome/Gecko: Render all content of ARIA treegrids in browse mode as a table.

* Gecko_ia2 bfubBackend: ensure that ARIA treegrids get a role of table as soon as possible, so that  it presents exactly like a table in brwose mode and quick nav to the table also works.

* Fix linting issue

* speech.getControlFieldSpeech: treeview items which are not normally spoken at all, should at least have expanded / collapsed / level announced.

* Gecko vbufBackend: split fetching of IAccessible2 attributes into its own function.

* Gecko vbufBackend: clarify comment

* Gecko vbufBackend: split checking for xml roles into its own function.

* Gecko vbufBackend: populateMapWithIA2AttributesFromPacc is now createMapOfIA2AttributesFromPacc, and returns a map.

* Added system test for ARIA treegrid in browse mode.

* Fix linting issues

* system test for ARIA treegrid: use the actual html file from the w3c ARIA practices repository, rather than a local copy, as it is very likely we will want to use more of these test cases in future.

* System tests: fix get_speech_at_index_until_now so that each line (sequence) is stripped of whitespace at its beginning and end. Previously whitespace was only stripped from the ends of the entire multiline string.

* Fix linting issues

* ARIA treegrid system test: focus a link in the iframe before moving to the treegrid so as to not have to deal with a possible focus event on the iframe document when switching to focus mode in the treegrid later.

* system tests: _chrome.getSpeechAfterKey: wait for any new speech to finish after sending a key.

* Update what's new.

.gitmodules

@@ -43,3 +43,6 @@
 [submodule "include/javaAccessBridge32"]
 	path = include/javaAccessBridge32
 	url = https://github.com/nvaccess/javaAccessBridge32-bin.git
+[submodule "include/w3c-aria-practices"]
+	path = include/w3c-aria-practices
+	url = https://github.com/w3c/aria-practices

nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp

@@ -33,6 +33,20 @@ This license can be found at:
 
 using namespace std;
 
+map<wstring,wstring> createMapOfIA2AttributesFromPacc(IAccessible2* pacc) {
+	map<wstring,wstring> IA2AttribsMap;
+	CComBSTR IA2Attributes;
+	if(pacc->get_attributes(&IA2Attributes) == S_OK) {
+		IA2AttribsToMap(IA2Attributes.m_str,IA2AttribsMap);
+	}
+	return IA2AttribsMap;
+}
+
+bool hasXmlRoleAttribContainingValue(const map<wstring,wstring>& attribsMap, const wstring roleName) {
+	const auto attribsMapIt = attribsMap.find(L"xml-roles");
+	return attribsMapIt != attribsMap.end() && attribsMapIt->second.find(roleName) != wstring::npos;
+}
+
 CComPtr<IAccessible2> GeckoVBufBackend_t::getLabelElement(IAccessible2_2* element) {
 	IUnknown** ppUnk=nullptr;
 	long nTargets=0;
@@ -422,6 +436,16 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(
 	nhAssert(parentNode); //new node must have been created
 	previousNode=NULL;
 
+	//get IA2Attributes -- IAccessible2 attributes;
+	map<wstring,wstring>::const_iterator IA2AttribsMapIt;
+	auto IA2AttribsMap = createMapOfIA2AttributesFromPacc(pacc);
+	// Add all IA2 attributes on the node
+	for(const auto& [key, val]: IA2AttribsMap) {
+		wstring attribName = L"IAccessible2::attribute_";
+		attribName += key;
+		parentNode->addAttribute(attribName, val);
+	}
+
 	//Get role -- IAccessible2 role
 	long role=0;
 	BSTR roleString=NULL;
@@ -438,6 +462,15 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(
 		else if(varRole.vt==VT_BSTR)
 			roleString=varRole.bstrVal;
 	}
+
+	// Specifically force the role of ARIA treegrids from outline to table.
+	// We do this very early on in the rendering so that all our table logic applies.
+	if(role == ROLE_SYSTEM_OUTLINE) {
+		if(hasXmlRoleAttribContainingValue(IA2AttribsMap, L"treegrid")) {
+			role = ROLE_SYSTEM_TABLE;
+		}
+	}
+
 	//Add role as an attrib
 	if(roleString)
 		s<<roleString;
@@ -495,22 +528,6 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(
 	} else
 		parentNode->addAttribute(L"keyboardShortcut",L"");
 
-	//get IA2Attributes -- IAccessible2 attributes;
-	BSTR IA2Attributes;
-	map<wstring,wstring> IA2AttribsMap;
-	if(pacc->get_attributes(&IA2Attributes)==S_OK) {
-		IA2AttribsToMap(IA2Attributes,IA2AttribsMap);
-		SysFreeString(IA2Attributes);
-		// Add each IA2 attribute as an attrib.
-		for(map<wstring,wstring>::const_iterator it=IA2AttribsMap.begin();it!=IA2AttribsMap.end();++it) {
-			s<<L"IAccessible2::attribute_"<<it->first;
-			parentNode->addAttribute(s.str(),it->second);
-			s.str(L"");
-		}
-	} else
-		LOG_DEBUG(L"pacc->get_attributes failed");
-	map<wstring,wstring>::const_iterator IA2AttribsMapIt;
-
 	//Check IA2Attributes, and or the role etc to work out if this object is a block element
 	bool isBlockElement=TRUE;
 	if(IA2States&IA2_STATE_MULTI_LINE) {
@@ -673,9 +690,10 @@ VBufStorage_fieldNode_t* GeckoVBufBackend_t::fillVBuf(
 	} else {
 		// If a node has children, it's visible.
 		isVisible = width > 0 && height > 0 || childCount > 0;
-		if ((role == ROLE_SYSTEM_LIST && !(states & STATE_SYSTEM_READONLY))
-			|| role == ROLE_SYSTEM_OUTLINE
-		) {
+		// Only render the selected item for interactive lists.
+		if (role == ROLE_SYSTEM_LIST && !(states & STATE_SYSTEM_READONLY)) {
+			renderSelectedItemOnly = true;
+		} else if(role == ROLE_SYSTEM_OUTLINE) {
 			renderSelectedItemOnly = true;
 		}
 		if (IA2TextIsUnneededSpace

source/NVDAObjects/IAccessible/ia2Web.py

@@ -112,6 +112,10 @@ class BlockQuote(Ia2Web):
 	role = controlTypes.ROLE_BLOCKQUOTE
 
 
+class Treegrid(Ia2Web):
+	role = controlTypes.ROLE_TABLE
+
+
 class Article(Ia2Web):
 	role = controlTypes.ROLE_ARTICLE
 
@@ -222,6 +226,8 @@ def findExtraOverlayClasses(obj, clsList, baseClass=Ia2Web, documentClass=None):
 	xmlRoles = obj.IA2Attributes.get("xml-roles", "").split(" ")
 	if iaRole == IAccessibleHandler.IA2_ROLE_SECTION and obj.IA2Attributes.get("tag", None) == "blockquote":
 		clsList.append(BlockQuote)
+	elif iaRole == oleacc.ROLE_SYSTEM_OUTLINE and "treegrid" in xmlRoles:
+		clsList.append(Treegrid)
 	elif iaRole == oleacc.ROLE_SYSTEM_DOCUMENT and xmlRoles[0] == "article":
 		clsList.append(Article)
 	elif xmlRoles[0] == "region" and obj.name:

source/speech/__init__.py

@@ -1910,6 +1910,18 @@ def getControlFieldSpeech(  # noqa: C901
 		out = []
 		if ariaCurrent:
 			out.extend(ariaCurrentSequence)
+		# Speak expanded / collapsed / level for treeview items (in ARIA treegrids)
+		if role == controlTypes.ROLE_TREEVIEWITEM:
+			if controlTypes.STATE_EXPANDED in states:
+				out.extend(
+					getPropertiesSpeech(reason=reason, states={controlTypes.STATE_EXPANDED}, _role=role)
+				)
+			elif controlTypes.STATE_COLLAPSED in states:
+				out.extend(
+					getPropertiesSpeech(reason=reason, states={controlTypes.STATE_COLLAPSED}, _role=role)
+				)
+			if levelSequence:
+				out.extend(levelSequence)
 		if role == controlTypes.ROLE_GRAPHIC and content:
 			out.append(content)
 		types.logBadSequenceTypes(out)

source/virtualBuffers/gecko_ia2.py

@@ -94,8 +94,7 @@ def _normalizeControlField(self,attrs):
 			# This is a named link destination, not a link which can be activated. The user doesn't care about these.
 			role=controlTypes.ROLE_TEXTFRAME
 		level=attrs.get('IAccessible2::attribute_level',"")
-
-		xmlRoles=attrs.get("IAccessible2::attribute_xml-roles", "").split(" ")
+		xmlRoles = attrs.get("IAccessible2::attribute_xml-roles", "").split(" ")
 		landmark = next((xr for xr in xmlRoles if xr in aria.landmarkRoles), None)
 		if landmark and role != controlTypes.ROLE_LANDMARK and landmark != xmlRoles[0]:
 			# Ignore the landmark role

tests/system/libraries/ChromeLib.py

@@ -125,6 +125,7 @@ def getSpeechAfterKey(key) -> str:
 		spy.wait_for_speech_to_finish()
 		nextSpeechIndex = spy.get_next_speech_index()
 		spy.emulateKeyPress(key)
+		spy.wait_for_speech_to_finish()
 		speech = spy.get_speech_at_index_until_now(nextSpeechIndex)
 		return speech
 

tests/system/libraries/SystemTestSpy/speechSpyGlobalPlugin.py

@@ -77,17 +77,6 @@ def _onNvdaSpeech(self, speechSequence=None):
 			self._lastSpeechTime_requiresLock = _timer()
 			self._nvdaSpeech_requiresLock.append(speechSequence)
 
-	@staticmethod
-	def _flattenCommandsSeparatingWithNewline(commandArray):
-		"""
-		Flatten many collections of speech sequences into a single speech sequence. Each original speech sequence
-		is separated by a newline string.
-		@param commandArray: is a collection of speechSequences
-		@return: speechSequence
-		"""
-		f = [c for commands in commandArray for newlineJoined in [commands, [u"\n"]] for c in newlineJoined]
-		return f
-
 	@staticmethod
 	def _getJoinedBaseStringsFromCommands(speechCommandArray) -> str:
 		baseStrings = [c for c in speechCommandArray if isinstance(c, str)]
@@ -103,11 +92,10 @@ def get_speech_at_index_until_now(self, speechIndex: int) -> str:
 		@return: The speech joined together, see L{_getJoinedBaseStringsFromCommands}
 		"""
 		with threading.Lock():
-			speechCommands = self._flattenCommandsSeparatingWithNewline(
-				self._nvdaSpeech_requiresLock[speechIndex:]
-			)
-			joined = self._getJoinedBaseStringsFromCommands(speechCommands)
-			return joined
+			speechCommands = [
+				self._getJoinedBaseStringsFromCommands(x) for x in self._nvdaSpeech_requiresLock[speechIndex:]
+			]
+			return "\n".join(x for x in speechCommands if x and not x.isspace())
 
 	def get_last_speech_index(self) -> int:
 		with threading.Lock():

tests/system/robot/chromeTests.py

@@ -6,6 +6,7 @@
 """Logic for NVDA + Google Chrome tests
 """
 
+import os
 from robot.libraries.BuiltIn import BuiltIn
 # imported methods start with underscore (_) so they don't get imported into robot files as keywords
 from SystemTestSpy import (
@@ -15,12 +16,18 @@
 # Imported for type information
 from ChromeLib import ChromeLib as _ChromeLib
 from AssertsLib import AssertsLib as _AssertsLib
+import NvdaLib as _NvdaLib
 
 _builtIn: BuiltIn = BuiltIn()
 _chrome: _ChromeLib = _getLib("ChromeLib")
 _asserts: _AssertsLib = _getLib("AssertsLib")
 
 
+ARIAExamplesDir = os.path.join(
+	_NvdaLib._locations.repoRoot, "include", "w3c-aria-practices", "examples"
+)
+
+
 def checkbox_labelled_by_inner_element():
 	_chrome.prepareChrome(
 		r"""
@@ -97,7 +104,7 @@ def announce_list_item_when_moving_by_word_or_character():
 	_asserts.strings_match(
 		actualSpeech,
 		"\n".join([
-			"list item  level 1  ",
+			"list item  level 1",
 			"b"
 		])
 	)
@@ -204,7 +211,7 @@ def test_pr11606():
 	_asserts.strings_match(
 		actualSpeech,
 		"\n".join([
-			"out of link  ",
+			"out of link",
 			"space"
 		])
 	)
@@ -222,3 +229,62 @@ def test_pr11606():
 		actualSpeech,
 		"bullet  link  A    link  B"
 	)
+
+
+def test_ariaTreeGrid_browseMode():
+	"""
+	Ensure that ARIA treegrids are accessible as a standard table in browse mode.
+	"""
+	testFile = os.path.join(ARIAExamplesDir, "treegrid", "treegrid-1.html")
+	_chrome.prepareChrome(
+		f"""
+			<iframe src="{testFile}" />
+		"""
+	)
+	# Jump to the first heading in the iframe.
+	actualSpeech = _chrome.getSpeechAfterKey("h")
+	_asserts.strings_match(
+		actualSpeech,
+		"frame  main landmark  Treegrid Email Inbox Example  heading  level 1"
+	)
+	# Tab to the first link.
+	# This ensures that focus is totally within the iframe
+	# so as to not cause focus to hit the iframe's document
+	# when entering focus mode on the treegrid later.
+	actualSpeech = _chrome.getSpeechAfterKey("tab")
+	_asserts.strings_match(
+		actualSpeech,
+		"issue 790.  link"
+	)
+	# Jump to the ARIA treegrid with the next table quicknav command.
+	# The browse mode caret will be inside the table on the caption before the first row.
+	actualSpeech = _chrome.getSpeechAfterKey("t")
+	_asserts.strings_match(
+		actualSpeech,
+		"Inbox  table  clickable  with 5 rows and 3 columns  Inbox"
+	)
+	# Move past the caption onto row 1 with downArrow
+	actualSpeech = _chrome.getSpeechAfterKey("downArrow")
+	_asserts.strings_match(
+		actualSpeech,
+		"row 1  Subject  column 1  Subject"
+	)
+	# Navigate to row 2 column 1 with NVDA table navigation command
+	actualSpeech = _chrome.getSpeechAfterKey("control+alt+downArrow")
+	_asserts.strings_match(
+		actualSpeech,
+		"expanded  level 1  row 2  Treegrids are awesome"
+	)
+	# Press enter to activate NVDA focus mode and focus the current row
+	actualSpeech = _chrome.getSpeechAfterKey("enter")
+	_asserts.strings_match(
+		actualSpeech,
+		"\n".join([
+			# focus mode turns on
+			"Focus mode",
+			# Focus enters the ARIA treegrid (table)
+			"Inbox  table",
+			# Focus lands on row 2
+			"level 1  Treegrids are awesome Want to learn how to use them? aaron at thegoogle dot rocks  expanded",
+		])
+	)

tests/system/robot/chromeTests.robot

@@ -35,3 +35,6 @@ i7562
 pr11606
 	[Documentation]	Announce the correct line when placed at the end of a link at the end of a list item in a contenteditable
 	test_pr11606
+ARIA treegrid
+	[Documentation]	Ensure that ARIA treegrids are accessible as a standard table in browse mode.
+	test_ariaTreeGrid_browseMode

tests/system/robot/startupShutdownNVDA.py

@@ -48,8 +48,10 @@ def quits_from_keyboard():
 
 	_asserts.strings_match(
 		actualSpeech,
-		"Exit NVDA  dialog  \n"
-		"What would you like to do?  combo box  Exit  collapsed  Alt plus d"
+		"\n".join([
+			"Exit NVDA  dialog",
+			"What would you like to do?  combo box  Exit  collapsed  Alt plus d"
+		])
 	)
 	_builtIn.sleep(1)  # the dialog is not always receiving the enter keypress, wait a little longer for it
 	spy.emulateKeyPress("enter", blockUntilProcessed=False)
@@ -65,13 +67,17 @@ def read_welcome_dialog():
 
 	_asserts.strings_match(
 		actualSpeech,
-		"Welcome to NVDA  dialog  Welcome to NVDA! Most commands for controlling NVDA require you to hold "
-		"down the NVDA key while pressing other keys. By default, the numpad Insert and main Insert keys "
-		"may both be used as the NVDA key. You can also configure NVDA to use the Caps Lock as the NVDA "
-		"key. Press NVDA plus n at any time to activate the NVDA menu. From this menu, you can configure "
-		"NVDA, get help and access other NVDA functions.  \n"
-		"Options  grouping  \n"
-		"Keyboard layout:  combo box  desktop  collapsed  Alt plus k"
+		"\n".join([
+			(
+				"Welcome to NVDA  dialog  Welcome to NVDA! Most commands for controlling NVDA require you to hold "
+				"down the NVDA key while pressing other keys. By default, the numpad Insert and main Insert keys "
+				"may both be used as the NVDA key. You can also configure NVDA to use the Caps Lock as the NVDA "
+				"key. Press NVDA plus n at any time to activate the NVDA menu. From this menu, you can configure "
+				"NVDA, get help and access other NVDA functions."
+			),
+			"Options  grouping",
+			"Keyboard layout:  combo box  desktop  collapsed  Alt plus k"
+		])
 	)
 	_builtIn.sleep(1)  # the dialog is not always receiving the enter keypress, wait a little longer for it
 	spy.emulateKeyPress("enter")

user_docs/en/changes.t2t

@@ -40,6 +40,7 @@ What's New in NVDA
 - NVDA should no longer freeze when waking the computer and focus lands in a Microsoft Edge document. (#11576)
 - It is no longer necessary to press tab or move focus after closing a context menu in MS Edge for browse mode to be functional again. (#11202)
 - NVDA no longer fails to read items in list views within a 64-bit application such as Tortoise SVN. (#8175)
+- ARIA treegrids are now exposed as normal tables in browse mode in both Firefox and Chrome. (#9715)
 
 
 == Changes for Developers ==