659886 - Element that changes between off-screen and on-screen keeps getting new accessibles

Status: RESOLVED FIXED

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

Description

[James Teh [:Jamie]]

Attachment: Test case.

If there is an element that is off-screen but then becomes on-screen, a new accessible is generated for the element instead of transforming the existing one. This is particularly problematic if the element becomes on-screen when it gains focus because if an AT explicitly set focus to it, the accessible that receives focus isn't the same as the one to which focus was set.

The attached test case includes two links which are off-screen by default but become on-screen when focused.

Str:
1. Open the attached test case.
2. Obtain the accessible for the "1" link.
3. Set focus to that accessible.
Expected: The accessible to which you set focus should be the accessible that receives focus.
Actual: The accessible to which you set focus dies and a new one (with a different unique ID) receives focus.

This is causing the following issue reported by Marco via email:
> The problem is that, on certain websites or under certain circumstances, braille loses contact to the virtual buffers and behaves as if NVDA was in focus mode or virtual buffer was turned off.

Marco's real world test case:
> To reproduce:
> 1. Open Firefox and go to http://www.wohnquartier-am-osterbekkanal.de/ausstattung.html
> 2. The issue should become visible as soon as NVDA hits the first or second of the skip links at the top of the page. Braille will shift to a structured view of the focused element and its ancestors, speech will continue to operate in virtual buffer mode.

Comment 1

[Marco Zehe (:MarcoZ)]

Thanks for looking into this, jamie! I'm actually seeing this in quite a number of places, like Facebook and other pages/web apps, since this is a common technique for Skip Links, for example.

Surkov, didn't we have a test that actually catches that mutations of different styling don't cause us to recreate accessibles? Why is this testcase different?

Comment 2

[alexander :surkov (:asurkov)]

(In reply to comment #1)

> Surkov, didn't we have a test that actually catches that mutations of
> different styling don't cause us to recreate accessibles? Why is this
> testcase different?

style changes may result in frame recreation (and as consequence accessible recreation), we have a case for overflow style change, we don't have for positioning changes.

Boris, can you look please at testcase and tell if frame recreation (nsCSSFrameConstructor::ContentRangeInserted/ContentRemoved) is necessary in this case?

Comment 3

[Boris Zbarsky [:bzbarsky]]

Yes, because the position changes which changes the frame tree in general (e.g. if the anchors had positioned descendants they would need to be parented to a different frame due to the position change).

Comment 4

[Marco Zehe (:MarcoZ)]

Alex, Jamie, we've been pinged about this issue again, and it still is reproducible in 62 nightly builds. See related NVDA issue https://github.com/nvaccess/nvda/issues/5247.

Comment 5

[alexander :surkov (:asurkov)]

Since the tendency nowadays to rely on HTML semantics instead a frame tree, I think the easiest way would be is to stop watching generic frame recreation notifications, and listen instead to some layout changes like display and visibility style changes, plus DOM mutations of course. We have few things to sort out like bug 1380451, before we can do this though.

Comment 6

[James Teh [:Jamie]]

What Alex said. :)

Comment 7

[Marco Zehe (:MarcoZ)]

Related testcase, distilled down from a shadow DOM testcase by Heydon Pickering:

data:text/html,<style>[aria-expanded="true"]:before {content:
'\25be\0020';}"</style><button aria-expanded="false"
onClick="this.setAttribute('aria-expanded', 'true');">Some info</button>

Comment 8

[James Teh [:Jamie]]

NVDA just had to back out a fix which stopped it from ignoring certain focus events because of this bug. It seems links to search results on Google get new accessibles when they're clicked. In addition, focusing certain buttons on GitHub causes them to get new accessibles. This was resulting in extraneous verbosity, hence the backout.
https://github.com/nvaccess/nvda/pull/8869#issuecomment-440443413

Ironically, the change was originally made precisely because of this bug to work around the fact that we generate new accessibles for cases like comment 7 and thus fire a focus change instead of a state change.

We really need to fix this.

Comment 9

[Marco Zehe (:MarcoZ)]

Attachment: Bug 659886 Part 1 - Switch content removals to be scheduled instead of immediate

In order for us to avoid recreating accessibles due to reframing, we must queue removals of accessibles instead of removing them immediately. Store the nodes to be removed in a hashtable.

Comment 10

[Marco Zehe (:MarcoZ)]

Attachment: Bug 659886 Part 2 - Don't remove an accessible and its subtree if it is being scheduled for insertion also.

When we schedule a range for insertion, we look to see if one of the nodes is also in the remoals queue. If that's the case, remove it from that queue, but let content insertion take its course anyway.

Depends on D22088

Comment 11

[James Teh [:Jamie]]

This is now being dealt with by Eitan in bug 686400 (which is broader than off-screen/on-screen as covered by this bug).

Comment 12

[James Teh [:Jamie]]

Confirmed that this is fixed by bug 686400.