Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't announce 'selected' when the focus moves in Google sheets if the focused cell is the only cell selected #8879

Merged
merged 5 commits into from
Oct 30, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions source/NVDAObjects/IAccessible/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
#This file is covered by the GNU General Public License.
#See the file COPYING for more details.

from comtypes.automation import IEnumVARIANT, VARIANT
from comtypes import COMError, IServiceProvider, GUID
from comtypes.hresult import S_OK
import ctypes
import os
import re
Expand Down Expand Up @@ -1214,6 +1216,38 @@ def _get_rowHeaderText(self):
def _get_columnHeaderText(self):
return self._tableHeaderTextHelper("column")

def _get_selectionContainer(self):
if self.table:
return self.table
feerrenrut marked this conversation as resolved.
Show resolved Hide resolved

def getSelectedItemsCount(self,maxCount):
# To fetch the number of selected items, we first try MSAA's accSelection, but if that fails in any way, we fall back to using IAccessibleTable2's nSelectedCells, if we are on an IAccessible2 table.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest splitting this into two internal helper methods:

  • _getSelectedItemsCount_MSAA
  • _getSelectedItemsCount_IAccesibleTable2

This way the the return paths can be simplified, and the two approaches are more clearly separated.

# Currently Chrome does not implement accSelection, thus for Google Sheets we must use nSelectedCells when on a table.
try:
sel=self.IAccessibleObject.accSelection
except COMError as e:
log.debugWarning("Error fetching accSelection, %s"%e)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is going to be fairly common (and expected in chrome, perhaps it's worth lowing the level to debug)

sel=None
if sel:
try:
enumObj=sel.QueryInterface(IEnumVARIANT)
except COMError as e:
enumObj=None
if enumObj:
numItemsFetched=ctypes.c_ulong()
itemsBuf=(VARIANT*(maxCount+1))()
res=enumObj._IEnumVARIANT__com_Next(maxCount,itemsBuf,ctypes.byref(numItemsFetched))
if res!=S_OK:
return numItemsFetched.value if numItemsFetched.value<=maxCount else sys.maxint
else:
log.debugWarning("Error in IEnumVARIANT::Next, code %s"%res)
if self.IAccessibleTable2Object:
try:
return self.IAccessibleTable2Object.nSelectedCells
except COMError as e:
log.debugWarning("Error calling IAccessibleTable2::nSelectedCells, %s"%e)
return 0

def _get_table(self):
if not isinstance(self.IAccessibleObject,IAccessibleHandler.IAccessible2):
return None
Expand Down
12 changes: 12 additions & 0 deletions source/NVDAObjects/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1181,3 +1181,15 @@ def _get__hasNavigableText(self):
return True
else:
return False

def _get_selectionContainer(self):
""" An ancestor NVDAObject which manages the selection for this object and other descendants."""
return None

def getSelectedItemsCount(self,maxCount=2):
"""
Fetches the number of descendants currently selected.
For performance, this method will only count up to the given maxCount number, and if there is one more above that, then sys.maxint is returned stating that many items are selected.
"""
return 0

2 changes: 1 addition & 1 deletion source/controlTypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,7 @@ def processNegativeStates(role, states, reason, negativeStates=None):
# but only if it is either focused or this is something other than a change event.
# The condition stops "not selected" from being spoken in some broken controls
# when the state change for the previous focus is issued before the focus change.
if role in (ROLE_LISTITEM, ROLE_TREEVIEWITEM, ROLE_TABLEROW) and STATE_SELECTABLE in states and (reason != REASON_CHANGE or STATE_FOCUSED in states):
if role in (ROLE_LISTITEM, ROLE_TREEVIEWITEM, ROLE_TABLEROW,ROLE_TABLECELL,ROLE_TABLECOLUMNHEADER,ROLE_TABLEROWHEADER) and STATE_SELECTABLE in states and (reason != REASON_CHANGE or STATE_FOCUSED in states):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you split this into multiple lines?

speakNegatives.add(STATE_SELECTED)
# Restrict "not checked" in a similar way to "not selected".
if (role in (ROLE_CHECKBOX, ROLE_RADIOBUTTON, ROLE_CHECKMENUITEM) or STATE_CHECKABLE in states) and (STATE_HALFCHECKED not in states) and (reason != REASON_CHANGE or STATE_FOCUSED in states):
Expand Down
9 changes: 9 additions & 0 deletions source/speech.py
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,15 @@ def speakObjectProperties(obj,reason=controlTypes.REASON_QUERY,index=None,**allo
newPropertyValues['current']=obj.isCurrent
if allowedProperties.get('placeholder', False):
newPropertyValues['placeholder']=obj.placeholder
# When speaking an object due to a focus change, the 'selected' state should not be reported if only one item is selected.
# This is because that one item will be the focused object, and saying selected is redundant.
# Rather, 'unselected' will be spoken for an unselected object if 1 or more items are selected.

states=newPropertyValues.get('states')
if states is not None and reason==controlTypes.REASON_FOCUS:
if controlTypes.STATE_SELECTABLE in states and controlTypes.STATE_SELECTED in states and obj.selectionContainer and obj.selectionContainer.getSelectedItemsCount(2)==1:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you split these conditions up over multiple lines please.

states.discard(controlTypes.STATE_SELECTED)
states.discard(controlTypes.STATE_SELECTABLE)
#Get the speech text for the properties we want to speak, and then speak it
text=getSpeechTextForProperties(reason,**newPropertyValues)
if text:
Expand Down