1515186 - IAccessible group information is not updated if an element is removed from a set with more than 2 items

Status: RESOLVED FIXED

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

Description

[derek.riemer]

Attachment: listDontDel.html

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/71.0.3578.98 Safari/537.36

Steps to reproduce:

1. open this test page.
2. focus the list with NVDA or jaws running.
3. move down the items, and verify that the item counts either screen reader reports are correct.
4. press d on an item.
5. An item is removed from the list.
6. Try moving around the list.


Actual results:

The info provided to screen readers on windows is incorrect.
The IAccessible object returns GroupPosition information that is incorrect, I.E. it says there are 6 items, not 5. This can be verified in  NVDA by focusing a list item, and putting this in the NVDA python console.
focus.IAccessibleObject.groupPosition

it should return (0, 5, k) and actually returns (0, 6, k)


Expected results:

It should provide the correct information to the screen reader.

Comment 1

[James Teh [:Jamie]]

Confirmed. display: inline-block isn't the cause, though. It seems that removing an item from a set that has more than 2 items is enough:

data:text/html,<div role="listbox"><div id="a" role="option">a</div><div id="b" role="option">b</div><div role="option">c</div></div><button onClick="a.remove();">Remove</button>

This is a listbox with three items. Pressing the Remove button removes the first item. The remaining two items get posinset updated correctly, but not setsize; setsize is still 3.

I guess our tree mutation code isn't marking the GroupInfo cache on the children as dirty in this case? I don't know why, though.

Comment 2

[James Teh [:Jamie]]

Oh. When we mark group info as dirty, we just set a flag on the accessible, but we don't clear mBits.groupInfo. Then, AccGroupInfo comes along and uses mBits.groupInfo on sibling accessibles without checking the flag. AccGroupInfo should always check the dirty flag before using mBits.groupInfo on a sibling.

Comment 3

[Marco Zehe (:MarcoZ)]

OK this is ... interesting. If I add a check to the if statement in https://dxr.mozilla.org/mozilla-central/source/accessible/base/AccGroupInfo.cpp#96 to check not only if the groupInfo for the (next) sibling is present but also if that is not dirty, removing the first list item in Derek's test case works. Also this enables Jamie's test case to work. But removing an item from the middle or end, this change does not cause an update of the setsize property. Also adding a check further up that file where the previous children are checked, to check whether that is also not dirty, does not fix this. Likewise, if I add a call to set the dirty flag on children in the Accessible::RemoveChild method, does not fix this.

Comment 4

[Marco Zehe (:MarcoZ)]

The reason was that our code assumed that if you had a, b, c, d as children, then removed b, c and d would be marked as having dirty group info, but a wouldn't. That is totally not true, since the removal of b changes the necessity for a to update its group info as well.

Got a working patch, now writing the test.

Comment 5

[Marco Zehe (:MarcoZ)]

Attachment: Bug 1515186 - Always calculate group position for all children of an accessible after a tree mutation, r=Jamie

Previously, if we had children a, b, c, and d, then removed b, the group position for c and d would potentially be marked as dirty, but a would not. This caused the check for the availability of previous group info to return outdated information.

This patch now always forces the update of all children's group position when a children move has occurred, since it potentially affects all the children, not just the ones after it. In addition, accGroupInfo::Update() now checks if the previous and next siblings that are being used as shortcuts have dirty group info, and are being used only if they do not.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/74bb778f7879
Always calculate group position for all children of an accessible after a tree mutation, r=Jamie

Comment 7

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/74bb778f7879