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

Allow table navigation commands in Google Docs #7946

Merged
merged 2 commits into from Mar 6, 2018

Conversation

Projects
None yet
3 participants
@michaelDCurran
Copy link
Contributor

michaelDCurran commented Jan 29, 2018

Link to issue number:

None

Summary of the issue:

Google Docs allows for tables in documents. Currently when arrowing around these documents with Google Docs braille mode enabled, NVDA announces these tables. However, it does not allow the user to perform NVDA table navigation commands (such as control+alt+arrows).

Description of how this pull request fixes the issue:

The IA2Web Editor NVDAObject now inherits also from documentBase.DocumentWithTableNavigation, and also implements _getTableCellAt. This allows table navigation to work for any editable content within an IAccessible2 web implementation (Firefox, Chrome etc).
Note that this is with focus mode which is the usual way a user would interact with Google Docs. Table navigation already works in browse mode.

Testing performed:

Created a document in Google Docs. Inserted a table. Arrows into the table, and then performed table navigation commands (up, down, left right).

Known issues with pull request:

None

Change log entry:

New features:

  • NVDA table navigation commands are now supported in Google Docs (with Braille mode enabled).

@michaelDCurran michaelDCurran requested a review from feerrenrut Jan 29, 2018

try:
cell = table.IAccessibleTable2Object.cellAt(destRow - 1, destCol - 1).QueryInterface(IAccessibleHandler.IAccessible2)
except AttributeError:
cell = table.IAccessibleTableObject.accessibleAt(destRow - 1, destCol - 1).QueryInterface(IAccessibleHandler.IAccessible2)

This comment has been minimized.

@feerrenrut

feerrenrut Jan 30, 2018

Contributor

is this guaranteed to succeed?

This comment has been minimized.

@michaelDCurran

michaelDCurran Jan 30, 2018

Author Contributor

It is guaranteed not to throw AttributeError. But it may throw COMError. The reason for catching AttributeError above is that IAccessibleTable2Object may not exist. However, at least one of IAccessibleTable2Object or IAccessibleTableObject will exist.

This comment has been minimized.

@feerrenrut

feerrenrut Jan 30, 2018

Contributor

Maybe it's assumed knowledge, but could you add a comment here to point that out?

if cell.IA2Attributes.get('hidden'):
raise LookupError("Found hidden cell")
return self.makeTextInfo(cell)
except (COMError, RuntimeError):

This comment has been minimized.

@feerrenrut

feerrenrut Jan 30, 2018

Contributor

Is there any way that you can narrow the scope of this try-except for COMError or RuntimeError. Its not apparent to me what will raise these?

This comment has been minimized.

@michaelDCurran

michaelDCurran Jan 30, 2018

Author Contributor

Any of the calls in the above try block could throw COMError. I think that makeTextInfo could throw RuntimeError... this was partly copied from virtualBuffers/gecko_ia2.py.

cell = table.IAccessibleTableObject.accessibleAt(destRow - 1, destCol - 1).QueryInterface(IAccessibleHandler.IAccessible2)
cell = IAccessible(IAccessibleObject=cell, IAccessibleChildID=0)
if cell.IA2Attributes.get('hidden'):
raise LookupError("Found hidden cell")

This comment has been minimized.

@feerrenrut

feerrenrut Jan 30, 2018

Contributor

How should this be handled? Is this left over debugging? If its not debugging, perhaps a comment to say what this might mean for those catching the exception.

@feerrenrut

This comment has been minimized.

Copy link
Contributor

feerrenrut commented Jan 30, 2018

An issue raised asking for table navigation in Google Docs: #7942 which suggests that one must swap to browse mode. This PR doesn't make it clear if table navigation is available in focus mode / browse mode / or both, could you mention that in the description?

@michaelDCurran

This comment has been minimized.

Copy link
Contributor Author

michaelDCurran commented Jan 31, 2018

@feerrenrut: I put a note in the description. This is for focus mode. Browse mode should already work, though that is not the default way a user would interact with Google docs. Especially not when creating content.

@michaelDCurran michaelDCurran requested a review from feerrenrut Jan 31, 2018

michaelDCurran added a commit that referenced this pull request Feb 7, 2018

@michaelDCurran michaelDCurran merged commit ea88eec into master Mar 6, 2018

@nvaccessAuto nvaccessAuto removed the incubating label Mar 6, 2018

@nvaccessAuto nvaccessAuto added this to the 2018.1 milestone Mar 6, 2018

@michaelDCurran michaelDCurran modified the milestones: 2018.1, 2018.2 Mar 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.