1663796 - Fields not removed from NVDA virtual buffer when cancelling editing of a GitHub comment

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[James Teh [:Jamie]]

STR (with the NVDA screen reader):

1. Open a GitHub issue to which you have access to edit a comment.
2. Press the options button for a comment and choose Edit comment.
3. Press the Cancel button to cancel editing of the comment.
4. Examine the document using the arrow keys.
   • Expected: The text box, Update comment button and Cancel button for editing the comment should not be present.
   • Actual: They are still present, even though they've definitely been removed from Firefox's a11y tree.

mozregression suggests this is a regression introduced by bug 686400. :(

Comment 1

[James Teh [:Jamie]]

Extremely distilled test case showing the root cause:
data:text/html,<form><div id="a">a</div></form><button onclick="a.hidden = true;">
The problem is that we don't fire a text removed event when something gets CSS display: none (or DOM hidden = true, which is more or less the same). This works correctly when we instead remove from the DOM (element.remove()), set CSS visibility: hidden or set aria-hidden="true".

The above test case doesn't cause a problem for NVDA, since a reorder gets fired on the container. However, this next test case demonstrates the real problem:
data:text/html,<div role="main"><form><div id="a">a</div></form><div id="b" hidden>b</div></div><button onclick="a.hidden = true; b.hidden = false;">
In this case, we hide something, but we also show something inside the grandparent. This means we fire a reorder event on the grandparent, but nothing on the parent. (Internally, we fire an inner reorder, but that isn't fired to platform.) This means that NVDA's buffer doesn't know anything changed inside the parent. It does crawl the grandparent, but when it sees the parent, it thinks it doesn't need to do anything, since no event was fired on it.

Aside from the symptom described here, it's possible this could be the cause of NVDA in-process stability bugs like bug 1786676, bug 1737688 and bug 1691928. If we're not firing the right tree mutation events, it's reasonable that NVDA's buffer could get very confused.

Comment 2

[James Teh [:Jamie]]

Attachment: Bug 1663796: Make Accessible::AppendTextTo produce text even if an Accessible has no frame.

Otherwise, we fail to fire a text removed event when something becomes display: none.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6eb4e2bde9c3
Make Accessible::AppendTextTo produce text even if an Accessible has no frame. r=eeejay

Comment 4

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/6eb4e2bde9c3