Closed Bug 1515186 Opened 9 months ago Closed 8 months ago

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

Categories

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

64 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox66 --- fixed

People

(Reporter: derek.riemer, Assigned: MarcoZ)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Attached file 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.
Component: Untriaged → Disability Access
OS: Unspecified → Windows
Hardware: Unspecified → Desktop
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.
Status: UNCONFIRMED → NEW
Component: Disability Access → Disability Access APIs
Ever confirmed: true
Priority: -- → P3
Product: Firefox → Core
Summary: if display:inline-block is set, IAccessible group information is not updated → IAccessible group information is not updated if an element is removed from a set with more than 2 items
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.
OS: Windows → All
Hardware: Desktop → All

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.

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.

Assignee: nobody → mzehe
Status: NEW → ASSIGNED
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.
Attachment #9035341 - Attachment description: Bug 1515186 - Always calculate group position for all children of an accessible, r=Jamie → Bug 1515186 - Always calculate group position for all children of an accessible after a tree mutation, r=Jamie
Pushed by mzehe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/74bb778f7879
Always calculate group position for all children of an accessible after a tree mutation, r=Jamie
Status: ASSIGNED → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Duplicate of this bug: 1280509
Blocks: groupa11y
You need to log in before you can comment on or make changes to this bug.