Closed Bug 1502260 Opened 6 years ago Closed 6 years ago

Event not fired on table when row added but table ancestor is also reordered (AKA table rows going missing with NVDA alpha)

Categories

(Core :: Disability Access APIs, defect, P1)

All
Windows
defect

Tracking

()

RESOLVED FIXED
Tracking Status
firefox-esr60 --- wontfix
firefox63 --- wontfix
firefox64 --- fixed
firefox65 --- fixed

People

(Reporter: Jamie, Unassigned)

References

Details

Huge thanks to Mick for his invaluable help in diagnosing this.

STR (with a current NVDA alpha build):
1. Load this test case:
data:text/html,<table><tbody id="tbody"><tr><th>a</th></tr></tbody></table><button onClick="tr = document.createElement('tr'); tr.innerHTML = '<td>b</td>'; tbody.appendChild(tr); div = document.createElement('div'); div.innerHTML = 'foo'; document.body.appendChild(div);">Go</button>
2. Press the Go button.
3. Press control+home.
Expected: NVDA should say "table with 2 rows and 1 column"...
Actual: NVDA says "table with 1 rows and 1 columns"...
4. Press down arrow.
Expected: NVDA should say "row 2  b"
Actual: NVDA says "out of table"...
5. Refresh the buffer by pressing NVDA+f5.
6. Press control+home.
Observe: NVDA says "table with 2 rows and 1 column"... as expected.

This can be seen in Phabricator when adding/removing review comments. I've also seen it on ANZ Internet Banking (Australia), where an entire section of the page goes missing because it's inside a layout able. There have also been other scattered reports on social media of tables misbehaving with NVDA alpha + Firefox, such as this one:
https://twitter.com/leonarddr/status/1052132490821943296

The reason this occurs is that Gecko coalesces reorder events for a subtree. In this example:
1. The table row gets added.
2. Thus, the table is a candidate for a reorder event.
3. The div gets added to the document.
4. Thus, the document is a candidate for a reorder event.
5. During coalescence, Gecko discovers that the table and the document both want to fire reorders, but the document is an ancestor, so it discards the reorder for the table.
6. No other event is fired on the table, so NVDA (which now tries to eliminate pointless subtree re-renders) doesn't know that its children need to be examined.

This is only a problem for tables because tables don't support IAccessibleText (bug 1052866) and thus don't fire text change events. Other things do fire text change events, which Gecko doesn't coalesce for subtrees, so NVDA catches the text change event and knows it needs to examine children.

There are two possible solutions here:
1. Don't coalesce reorder events for subtrees (or at least for tables).
2. Go ahead with bug 1052866, though Alex still has outstanding objections.

Unfortunately, the known (and potential) impact of this bug on users means that the recent NVDA speed-ups for Firefox are at risk and may be backed out until this lands in a stable Firefox release. That may also mean they won't make it into NVDA 2018.4.
Summary: Reorder event not fired on table when row added but table ancestor is also reordered → Reorder event not fired on table when row added but table ancestor is also reordered (AKA table rows going missing with NVDA alpha)
Marco, would you mind testing with JAWS to see if it is affected? It's possible it uses show events (or re-renders the entire subtree), so it may not be. I consider this to be a bug regardless, but it'd be good to know anyway.
Flags: needinfo?(mzehe)
(In reply to James Teh [:Jamie] from comment #0)

> There are two possible solutions here:
> 1. Don't coalesce reorder events for subtrees (or at least for tables).
> 2. Go ahead with bug 1052866, though Alex still has outstanding objections.

I'd say let's go with bug 1052866, despite its downsides, it keeps things simpler from AT perspective. Also not coalescing events for table seems worse.
(In reply to James Teh [:Jamie] from comment #1)
> Marco, would you mind testing with JAWS to see if it is affected? It's
> possible it uses show events (or re-renders the entire subtree), so it may
> not be. I consider this to be a bug regardless, but it'd be good to know
> anyway.

JAWS is not affected. For one, it treats the table in the test case as a layout table. It uses its own heuristics and seems to ignore the layout-guess object attribute. And b) it always renders both a and b after I press the button. So it definitely catches the update.
Flags: needinfo?(mzehe)
(In reply to alexander :surkov (:asurkov) from comment #2)
> (In reply to James Teh [:Jamie] from comment #0)
> 
> > There are two possible solutions here:
> > 1. Don't coalesce reorder events for subtrees (or at least for tables).
> > 2. Go ahead with bug 1052866, though Alex still has outstanding objections.
> 
> I'd say let's go with bug 1052866, despite its downsides, it keeps things
> simpler from AT perspective. Also not coalescing events for table seems
> worse.

If I'm correct, another benefit of having IAccessibleText support for tables might be that it will improve NVDA's recent support to get points from offsets in virtual buffers (https://github.com/nvaccess/nvda/pull/8479)
I can confirm that applying the patch from bug 1052866 fixes the test case from comment #0. In addition, this also fixes the table coordination update problem from bug 1499519.
Depends on: 1052866
Now that bug 1052866 is fixed, a text change will be fired on the table when rows are added or removed. This isn't a reorder event as originally requested in this bug, but it is perfectly acceptable and makes this consistent with other (non-table) elements.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Summary: Reorder event not fired on table when row added but table ancestor is also reordered (AKA table rows going missing with NVDA alpha) → Event not fired on table when row added but table ancestor is also reordered (AKA table rows going missing with NVDA alpha)
You need to log in before you can comment on or make changes to this bug.