Closed Bug 1668701 Opened 4 years ago Closed 4 years ago

The searchfox position:sticky nesting divs break NVDA's understanding of the code as a table once nested 2 deep

Categories

(Webtools :: Searchfox, defect)

defect

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 1668707

People

(Reporter: asuth, Unassigned)

References

Details

While testing the latest revision of the coverage-adding bug in bug 1566874 [1] I encountered a situation where hitting ctrl-alt-down to navigate through the coverage column ended up wrapping around to row 1. I realized this was happening once encountering the divs we use to wrap the range that position:sticky lines of code are sticky and that this is a problem even in the current trunk.

These divs on trunk currently only have CSS classes associated. The accessibility devtools reports them as "section", but adding role="rowgroup" didn't change NVDA's behavior. But since I think the spec for ARIA rowgroup seems to imply rowgroup should only have rows, not more rowgroups, that could make sense. I also tried changing the role=table to role=grid (independent or the rowgroup change). That might leave treegrid as a more dramatic option?

The STR is:

  • Pick a file with position:sticky lines that create a nesting depth of at least 2. This is quite common in code that uses namespaces. For example https://searchfox.org/mozilla-central/source/dom/base/BarProps.h has namespace mozilla { on line 25 and namespace dom { on line 29. When scrolling beyond line 29, both namespace lines will stick to the viewport.
  • Use ctrl-alt-right to move to the line number column because it helps us double check that the problem isn't something like NVDA resetting the row number counter but otherwise doing the right thing.
  • Repeatedly hit "ctrl-alt-down". In my case, the transition was dictated by NVDA as:
    • Row 27 clickable 27.
    • Row 28 clickable 28.
    • Clickable 29.
    • Row 1 clickable 1.

Note that when entering the table NVDA seems to correctly know how many lines are in the table (128), which is interesting.

:Jamie, do you have any thoughts on whether this is something we should be mitigating in searchfox or Firefox or might be an NVDA bug?

1: new attempt up at https://asuth.searchfox.org/ with the coverage and blame aria-labels per discussion starting at https://bugzilla.mozilla.org/show_bug.cgi?id=1566874#c25

Flags: needinfo?(jteh)

I've been meaning to file a bug about this for a while. :)

This is a bug in Gecko, since intervening divs between a table and its rows should be ignored. We partially addressed it in bug 1455066, but only for one level of intervening div.

We should fix this in Gecko, but in the interim, it's probably faster (and results in a cleaner a11y tree) to have Searchfox put role="presentation" on these sticky (.nesting-container) divs, since they're not useful semantically anyway.

Flags: needinfo?(jteh)

I actually can't work out why the a11y engine is creating Accessibles for those intervening divs at all. We have code precisely to avoid creation of pointless Accessibles in that case. Guess I'll be doing some debugging. :(

Depends on: 1668707

(In reply to James Teh [:Jamie] from comment #2)

I actually can't work out why the a11y engine is creating Accessibles for those intervening divs at all.

Figured it out. I'll fix this in bug 1668707, which means we don't need any fix on the Searchfox side.

I've verified that the problem is solved on Firefox nightly for me on Windows running NVDA 2020.2, so duping to bug 1668707 since there's nothing more to do here.

I've also verified that when I removed the role="rowgroup" that I'd added to the ".nesting-container" divs from the coverage branch, the UX for the coverage and blame/annotate columns then both operated as expected/desired. (Also, the coverage branch corrects the blame popup so that hitting enter again removes the blame popup.)

Thanks very much for the quick fix and all your help here!

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.