I6358 support aria current #6860

feerrenrut · 2017-02-09T10:19:05Z

Support for aria current. Used http://ljwatson.github.io/design-patterns/aria-current/ to test this, as described we report just 'current' if True and 'current step' etc if there is another value.

Fixes #6358

While in focus mode, we can now get the values for aria-current rather than just the presence of the property.

feerrenrut · 2017-02-09T10:21:04Z

aria-current is not yet being reported for list items on any browser.

@jcsteh mentioned yesterday that:

List items are probably treated as layout by TextInfo.getPresentationCategory. A bit tricky because we don't want other states like read-only or the role to get reported, only really aria-current

jcsteh

Nice work so far. Thanks. :)

Aside from the review comments, support also needs to be implemented for braille. You'll want to look at braille.getBrailleTextForProperties, braille.NVDAObjectRegion.update and braille.getControlFieldBraille. The changes will be similar to those in speech. I realise you don't have a braille display, but you can use the Braille Viewer driver I hacked together some time ago to get braille enabled and then look at the IO log output to see the text being translated.

jcsteh · 2017-02-14T00:43:50Z

source/NVDAObjects/__init__.py

+	def getValueForAriaCurrent(self):
+		"""Gets the value for aria-current. Normally returns False. If this object is current
+		it will return one of the following values: True, "page", "step", "location", "date", "time"
+		"""


This should be a property rather than a method. Aside from consistency with other NVDAObject properties (name, description, etc.), this means this will be cached during a core tick which improves performance. In NVDA, you can do this by simply naming the method _get_foo, which will create a property named foo.

While this is from the ARIA spec, we try to keep global concepts pretty abstract/platform agnostic. This one is tricky to name because just having a property called current looks kinda weird. We feel the name isCurrent is probably best. While this is perhaps a tiny bit misleading (the "is" prefix normally suggests it's a boolean), it's the best we could come up with... and it is boolean-ish anyway in the sense that it's either current or not, with the type just being a nicety for the user.

The docstring should probably say that this indicates whether this object is the current element in a set of related elements... or something like that; something similar to the summary sentence in the ARIA spec. It can still say it's mapped from aria-current, though.

Yeah ok. I'm not 100% comfortable with the name, but I can't think of anything better.

jcsteh · 2017-02-14T00:47:55Z

source/NVDAObjects/UIA/edge.py

@@ -387,6 +389,16 @@ def _get_description(self):
 				pass
 		return super(EdgeNode,self).description

+	def getValueForAriaCurrent(self):
+		ariaProperties=self.UIAElement.currentAriaProperties
+		match = re.match("current=(\w+);", ariaProperties)


Please make this a compiled regular expression constant. See re.compile. We tend to name such constants RE_FOO.

jcsteh · 2017-02-14T00:49:04Z

source/speech.py

@@ -1001,6 +1002,20 @@ def getSpeechTextForProperties(reason=controlTypes.REASON_QUERY,**propertyValues
 	if rowCount or columnCount:
 		# The caller is entering a table, so ensure that it is treated as a new table, even if the previous table was the same.
 		oldTableID = None
+	ariaCurrent = propertyValues.get('current', False)
+	if ariaCurrent is not None and ariaCurrent != False:


You could simplify this to just: if ariaCurrent:

jcsteh · 2017-02-14T00:52:01Z

source/speech.py

+	ariaCurrent = propertyValues.get('current', False)
+	if ariaCurrent is not None and ariaCurrent != False:
+		if ariaCurrent=="page":
+			textList.append(_("current page"))


I'd suggest moving these names into a dict in controlTypes, perhaps named currentLabels.

Each of these translatable strings needs a translators comment.

jcsteh · 2017-02-14T01:09:18Z

source/speech.py

@@ -1112,7 +1129,7 @@ def getControlFieldSpeech(attrs,ancestorAttrs,fieldType,formatConfig=None,extraD
 		content = attrs.get("content")
 		if content and speakContentFirst:
 			out.append(content)
-		out.extend(x for x in (nameText,(stateText if speakStatesFirst else roleText),(roleText if speakStatesFirst else stateText),valueText,descriptionText,levelText,keyboardShortcutText) if x)
+		out.extend(x for x in (nameText,(stateText if speakStatesFirst else roleText),(roleText if speakStatesFirst else stateText),ariaCurrentText,valueText,descriptionText,levelText,keyboardShortcutText) if x)
 		if content and not speakContentFirst:
 			out.append(content)
 		return CHUNK_SEPARATOR.join(out)


Regarding speaking for list items, etc., this can be achieved by handling current similar to the way we handle clickable below. The conditions for current should be the same as clickable except we do want to report it even if extraDetail is True (whereas we don't for clickable). The tricky thing here is that ideally, we still want to report current if we're reporting clickable (and vice versa). So, I guess the clickable and current block will have to be merged.

jcsteh · 2017-02-14T01:09:39Z

source/virtualBuffers/__init__.py

@@ -257,6 +257,12 @@ def _getParagraphOffsets(self,offset):
 		return lineStart.value,lineEnd.value

 	def _normalizeControlField(self,attrs):
+


Extraneous blank line.

jcsteh · 2017-02-14T01:10:39Z

source/virtualBuffers/__init__.py

@@ -257,6 +257,12 @@ def _getParagraphOffsets(self,offset):
 		return lineStart.value,lineEnd.value

 	def _normalizeControlField(self,attrs):
+
+		ariaCurrent = attrs.get("IAccessible2::attribute_current")


Please move this into the gecko_ia2 virtual buffer.

jcsteh · 2017-02-14T01:11:38Z

source/virtualBuffers/__init__.py

+		ariaCurrent = attrs.get("IAccessible2::attribute_current")
+		if ariaCurrent != None:
+			attrs['current']= ariaCurrent
+			del attrs["IAccessible2::attribute_current"]


Probably no need to delete this; we don't do it with other attributes and leaving it there might be useful for debugging.

jcsteh · 2017-02-14T01:16:51Z

source/speech.py

@@ -1112,7 +1129,7 @@ def getControlFieldSpeech(attrs,ancestorAttrs,fieldType,formatConfig=None,extraD
 		content = attrs.get("content")
 		if content and speakContentFirst:
 			out.append(content)
-		out.extend(x for x in (nameText,(stateText if speakStatesFirst else roleText),(roleText if speakStatesFirst else stateText),valueText,descriptionText,levelText,keyboardShortcutText) if x)
+		out.extend(x for x in (nameText,(stateText if speakStatesFirst else roleText),(roleText if speakStatesFirst else stateText),ariaCurrentText,valueText,descriptionText,levelText,keyboardShortcutText) if x)


I think it's possible that someone might set aria-current on a table cell; e.g. in a calendar. So, it also needs to be reported for the table cell case above this.

- ensure that aria-current is output to braille. - make `getValueForAriaCurrent` a property called isCurrent - use a compiled regex - simplify `if` statement - move text labels for 'aria-current' values to a dictionary and translate - handle aria-current in list items and tables

feerrenrut · 2017-02-22T15:06:44Z

To test this (with clickable list items, and clickable table cells) I used the following HTML file (unzip first)
aria-current-test.zip

jcsteh

Looks good! Ta!

jcsteh · 2017-02-23T06:38:09Z

source/braille.py

@@ -630,9 +633,16 @@ def getBrailleTextForProperties(**propertyValues):
 			# Translators: Displayed in braille for a table cell column number.
 			# %s is replaced with the column number.
 			textList.append(_("c%s") % columnNumber)
+	ariaCurrent = propertyValues.get('current', False)


nit: Please rename to current.

jcsteh · 2017-02-23T06:38:34Z

source/braille.py

@@ -698,12 +708,16 @@ def getControlFieldBraille(info, field, ancestors, reportStart, formatConfig):
 	role = field.get("role", controlTypes.ROLE_UNKNOWN)
 	states = field.get("states", set())
 	value=field.get('value',None)
+	ariaCurrent=field.get('current', None)


Please rename.

jcsteh · 2017-02-23T06:41:07Z

source/controlTypes.py

@@ -613,6 +613,23 @@
 REASON_ONLYCACHE="onlyCache"
 #}

+# Text to use for 'current' values. These describe if an item is the current item 
+# within a particular kind of selection.


Please change the comment prefix to #::

#: l1 #: l2

This means it'll be treated as the documentation of the variable by epydoc (the code doc tool).

Incubate PR #6860 to fix issue #6358 Merge branch 'i6358-supportAriaCurrent' into next

For PR #6761 - Web page menu items (menu item checkbox's and radio buttons) can now be activated while in browse mode. Issue #6735 For PR #6866 - Excel sheet name reporting is now translated. Issue #6848 For PR #6884 - Pressing ESC while the configuration profile "Confirm Deletion" prompt is active now dismisses the dialog. Issue #6851 For PR #6895 - Cell border information can now be reported in Microsoft Excel by using `NVDA+f`. Issue #3044 For PR #6860 - Added support for aria-current attributes. Issue #6358

feerrenrut added 7 commits January 24, 2017 17:27

Support for aria-current in focus mode for chrome

258dc82

Support for aria-current in focus mode for IE11

7c41316

Add support for aria-current in focus mode to Edge

0b23b86

Added support for aria-current values with edge

75c28c6

While in focus mode, we can now get the values for aria-current rather than just the presence of the property.

aria current working in IE while in browse mode

8ff0d42

Aria-current now reported in browse mode for Edge

248e278

Support other values of aria-current

43bc534

jcsteh requested changes Feb 14, 2017

View reviewed changes

feerrenrut requested a review from jcsteh February 22, 2017 12:56

jcsteh approved these changes Feb 23, 2017

View reviewed changes

Further review actions for #6860

9be84bd

feerrenrut added a commit that referenced this pull request Feb 23, 2017

incubates #6860

af44c1d

Incubate PR #6860 to fix issue #6358 Merge branch 'i6358-supportAriaCurrent' into next

nvaccessAuto added the incubating label Feb 23, 2017

nvaccessAuto assigned feerrenrut Feb 23, 2017

feerrenrut merged commit 2fde87e into master Mar 14, 2017

nvaccessAuto removed the incubating label Mar 14, 2017

nvaccessAuto added this to the 2017.2 milestone Mar 14, 2017

feerrenrut deleted the i6358-supportAriaCurrent branch September 21, 2017 01:31

feerrenrut mentioned this pull request Jan 8, 2018

Handle aria-current=false as False instead of True #7901

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

I6358 support aria current #6860

I6358 support aria current #6860

feerrenrut commented Feb 9, 2017 •

edited

feerrenrut commented Feb 9, 2017

jcsteh left a comment

jcsteh Feb 14, 2017

feerrenrut Feb 14, 2017

jcsteh Feb 14, 2017

jcsteh Feb 14, 2017

jcsteh Feb 14, 2017

jcsteh Feb 14, 2017

jcsteh Feb 14, 2017

jcsteh Feb 14, 2017

jcsteh Feb 14, 2017

jcsteh Feb 14, 2017

feerrenrut commented Feb 22, 2017

jcsteh left a comment

jcsteh Feb 23, 2017

jcsteh Feb 23, 2017

jcsteh Feb 23, 2017

		@@ -257,6 +257,12 @@ def _getParagraphOffsets(self,offset):
		return lineStart.value,lineEnd.value

		def _normalizeControlField(self,attrs):

I6358 support aria current #6860

I6358 support aria current #6860

Conversation

feerrenrut commented Feb 9, 2017 • edited

feerrenrut commented Feb 9, 2017

jcsteh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feerrenrut commented Feb 22, 2017

jcsteh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feerrenrut commented Feb 9, 2017 •

edited