Ensure not to maintain AccessibleTable2 when fetching child nodes when the underlying table has neither rows nor columns#12360
Conversation
…n the underlying table has neither rows nor columns
What about Basilisk - it is quite popular in certain circles due to its compatibility with older style add-ons for Firefox and as far as I can see is still in active development? |
|
I tested it and it supports IAccessibleTable2 as well.
|
See test results for failed build of commit da1193ed69 |
|
Waterfox Classic as well?
Brian
***@***.***
Sent via blueyonder.
Please address personal E-mail to:-
***@***.***, putting 'Brian Gaff'
in the display name field.
Newsgroup monitored: alt.comp.blind-users
----- Original Message -----
From: "Łukasz Golonka" ***@***.***>
To: "nvaccess/nvda" ***@***.***>
Cc: "Subscribed" ***@***.***>
Sent: Monday, May 03, 2021 1:04 PM
Subject: Re: [nvaccess/nvda] Ensure not to maintain AccessibleTable2 when
fetching child nodes when the underlying table has neither rows nor columns
(#12360)
…> As a bonus, I removed support for IAccessibleTable2 support. This support
> was left in the VBuf code to support older versions of Seamonkey. I
> verified that Seamonkey now uses a version of Gecko that also uses
> IAccessibleTable2.
What about [Basilisk](https://www.basilisk-browser.org/) - it is quite
popular in certain circles due to its compatibility with older style
add-ons for Firefox and as far as I can see is still in active
development?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
#12360 (comment)
|
|
Feel free to try the try build with whatever you like. I think
IAccessibleTable2 has been supported for around a year of ten in Firefox
now. IAccessibleTable1 is really old.
|
|
A similar case that can be used to reproduce, is by embedding a table in a div with a role of grid. I know that this is bad authoring practice, but currently NVDA gets pretty frustrated by these cases, whereas this pr properly announces the inner table. This is also reproducible in Firefox. |
|
Note that current WaterFox is built on Gecko 78, but is not supported by NVDA's virtual buffers because its main window class is "WaterFoxWindowClass", not "MozillaWindowClass", which NVDA checks for. So if you try WaterFox with this build or any NVDA build really, you won't get virtual buffers, but that has nothing to do with this PR. |
| // Fetch row and column counts and add them as attributes on the vbuf node. | ||
| long rowCount = 0; | ||
| if (curNodePaccTable2->get_nRows(&rowCount) == S_OK) { | ||
| s << rowCount; | ||
| parentNode->addAttribute(L"table-rowcount", s.str()); | ||
| s.str(L""); | ||
| } | ||
| long colCount = 0; | ||
| if (curNodePaccTable2->get_nColumns(&colCount) == S_OK) { | ||
| s << colCount; | ||
| parentNode->addAttribute(L"table-columncount", s.str()); | ||
| s.str(L""); | ||
| } | ||
| if (rowCount > 0 || colCount > 0) { | ||
| // This table has rows and columns. | ||
| // Maintain curNodePaccTable2 for child rendering until any table cells are found. | ||
| paccTable2 = curNodePaccTable2; | ||
| } |
There was a problem hiding this comment.
Could this still be done in a helper function? It would be good to continue to split fillVBuf up.
There was a problem hiding this comment.
It could be done in a helper function theoretically, but I'm afraid it will make the code less readable that way. Note that I removed this from a helper function because when it was a helper, it only fetched the row and column count to write it to the vbuf xml. Now the row and column count are used later on as well.
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
Link to issue number:
Fixes #12359
Summary of the issue:
When creating a virtual buffer and NVDA encounters a table, it wil render the children of that table. It keeps a reference to the IAccessibleTable2 interface of the table until it has found a cell in that table. However, when a table contains no cells (i.e. when it has neither rows nor columns) and it encounters an inner table in that table, that inner table is ignored and table navigation does not work.
Description of how this pull request fixes the issue:
This pr ensures that when row and column count are 0, the IAccessibleTable2 reference isn't used when rendering children. Therefore, for every child, the logic that checks for IAccessibleTable2 will apply and issue #12359 no longer occurs.
As a bonus, I removed support for IAccessibleTable2 support. This support was left in the VBuf code to support older versions of Seamonkey. I verified that Seamonkey now uses a version of Gecko that also uses IAccessibleTable2.
Testing strategy:
Exampel attached to #12359 as wel as a private environment where this issue was prevalent.
Known issues with pull request:
None
Change log entries:
Bug fixes
Code Review Checklist: