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

Fix table navigation commands in Google Docs #9562

Merged
merged 5 commits into from May 15, 2019

Conversation

Projects
None yet
4 participants
@michaelDCurran
Copy link
Contributor

commented May 13, 2019

Link to issue number:

Fixes #9494

Summary of the issue:

Currently trying to move around a table in Google Docs with Firefox or Chrome, using NVDA's table navigation commands fails with an exception.
Specifically, NVDAObject's rowNumber, columnNumber etc expose web author specified rowindex and colindex ARIA values which don't work with the other IAccessible2 table methods for looking up cells.
For browseMode at least, this was already partly addressed by exposing special table-physical* attributes on controlFields which were used for table navigation, where as presentation (speech, braille) used the standard table-* attributes.

Description of how this pull request fixes the issue:

Rewrite how presentational verses physical table properties are exposed entirely. Instead exposing default / physical table properties as their standard rowNumber, columnNumber etc. If the underlying API does have presentational coordinates, then expose these as the presentational* values, using them only for presentation, and not physical table navigation.
Specific changes:

  • Expose the real IAccessible2 table coordinates for navigation via rowNumber, columnNumber, rowCount and columnCount NvDAObject properties, and controlField attributes, rather than IA2-specific table properties.
  • Add new optional properties to NVDAObjects: presentationalRowNumber, presentationalColumnNumber, presentationalRowCount and presentationalColumnCount.
  • Make use of these presentational* properties in speech if they are implemented, otherwise fall back to the standard properties.
  • Provide implementations of the presentational* properties for tables on NVDAObjects for IAccessible2 that uses rowindex and colindex IA2 aria attributes, allowing web authors to override how table coordinates are presented without confusing actual table navigation logic.
  • Similarly for controlFields, expose table-* and table-*-presentational attributes allowing for overriding of table coordinate presentation when the web author has specified it.
  • IA2TextInfo's updateSelection method: If the requested start and end offsets are the same (I.e. the caller wants a collapsed selection) call setCaretOffset instead. This gets around strange bugs in Google Chrome / Google Docs where making collapsed selections in table cells selects the entire cell.

Testing performed:

Using this testcase:
table.html.txt
Opened it in both Mozilla Firefox and Google Chrome.
Ensured that:

  • Arrowing into the first table, and then switching to focus mode, that NVDA's table navigation commands (control/alt+up/down/left/right) successfully moved around the table.
  • Ensured that when arrowing through the second table in browseMode, that table coordinates reported correctly. (The table is a standard table with no ARIA properties). Also ensured that table navigation commands still worked.
  • When arrowing into the 3rd table, ensured that table coordinates reported as expected. Although this table is only 3 rows by 2 columns, it should report as 20 rows by 20 columns, columns should be 10 and 11, and rows should be numbered: 1, 10 and 11. These coordinates have been specified using aria-rowindex etc.
  • Created a table in Google Docs with Braille mode on, and ensured that NVDA's table navigation commands successfully move around the table.

Known issues with pull request:

None.

Change log entry:

Bug fixes:

  • It is again possible to use NvDA's table navigation commands in Google Docs, in Firefox and Chrome.
Allow table navigation in Google Docs.
Specifically:
* Expose the real IAccessible2 table coordinates for navigation via rowNumber, columnNumber, rowCount and columnCount NvDAObject properties, and controlField attributes, rather than IA2-specific table properties.
Add new optional properties to NVDAObjects: presentationalRowNumber, presentationalColumnNumber, presentationalRowCount and presentationColumnCount.
* Provide implementations of the presentational* properties for tables on NVDAObjects for IAccessible2 that uses rowindex and colindex IA2 aria attributes, allowing web authors to override how table coordinates are presented without confusing actual table navigation logic.
* Similarly for controlFields, expose table-* and table-*-presentational attributes allowing for overriding of table coordinate presentation when the web author has specified it.
* IA2TextInfo's updateSelection method: If the requested start and end offsets are the same (I.e. the caller wants a collapsed selection) call setCaretOffset instead. This gets around strange bugs in Google Ghrome / Google Docs where making collapsed selections in table cells selects the entire cell.

@michaelDCurran michaelDCurran requested a review from feerrenrut May 13, 2019

Show resolved Hide resolved source/NVDAObjects/__init__.py Outdated
Show resolved Hide resolved source/NVDAObjects/__init__.py Outdated
Show resolved Hide resolved nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp Outdated
Show resolved Hide resolved source/NVDAObjects/__init__.py Outdated
@rtype: int
"""
raise NotImplementedError

def _get_rowSpan(self):

This comment has been minimized.

Copy link
@leonardder

leonardder May 13, 2019

Collaborator

Just to make sure, could there also be something like a presentational row/column span?

This comment has been minimized.

Copy link
@michaelDCurran

michaelDCurran May 15, 2019

Author Contributor

I'm not aware of any colspan or rowspan ARIA attributes at this stage. Only rowIndex and rowCount. Spanning would also have an affect on the physical table thus I don't think that could ever be virtualized.

Show resolved Hide resolved source/speech.py Outdated
Show resolved Hide resolved nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp Outdated
Show resolved Hide resolved nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp Outdated
Show resolved Hide resolved nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp Outdated
Show resolved Hide resolved source/NVDAObjects/IAccessible/__init__.py
Show resolved Hide resolved source/documentBase.py Outdated
@michaelDCurran

This comment has been minimized.

Copy link
Contributor Author

commented May 14, 2019

@feerrenrut I have addressed the rest of the review comments from you and @leonardder

@feerrenrut

This comment has been minimized.

Copy link
Contributor

commented May 15, 2019

I don't think it should block this PR, however there is still a question from @leonardder:

Just to make sure, could there also be something like a presentational row/column span?

@michaelDCurran michaelDCurran merged commit c20a503 into master May 15, 2019

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@nvaccessAuto nvaccessAuto added this to the 2019.2 milestone May 15, 2019

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.