jcsteh / osara

OSARA: Open Source Accessibility for the REAPER Application
GNU General Public License v2.0
122 stars 46 forks source link

can we do some light reshaping of how controls in the Peak Watcher GUI are named to shore up UX across screen readers? #861

Open ScottChesworth opened 1 year ago

ScottChesworth commented 1 year ago

Navigating the "Hold level" grouping of Peak Watcher's GUI reports differently with NVDA, JAWS and VoiceOver. While absolute consistency probably isn't possible, I'm thinking perhaps there's a way to clarify the UX with some light reshaping of language and one role tweak. Jotting down notes here best I can with a limited technical understanding. If someone can point me at where these controls are named, I'd be happy to sit with the screen readers and twiddle until I get some sort of improvement.

These are the current issues I'm aware of:

  1. The edit field that becomes navigable once our radiobutton in this group is set to "for (ms)" doesn't have an accName yet. This causes NVDA to just report "edit" (not a huge deal because what's expected there is only one control away), but with VO navigation that's less clear IMO, and JAWS decides to report "When level reaches" there which is just... er... wrong. Would adding an accName of "Hold for ms" solve this?
  2. I think forward and backward navigation through the tab order would be clearer with these language tweaks:
    • "Hold level" becomes "Hold level until:"
    • "until reset" becomes "the user resets this watcher"
    • "for (ms):" becomes "a given amount of time passes"
  3. Could we also add an accelerator to the edit field where ms is specified? H seems logical if we go with the accName suggestion above.

Obviously these suggestions make reports longer, but IMO the increased clarity for newcomers is a worthy trade. Power users are familiar enough with this GUI by now that I'm pretty sure it won't slow them down.

Thoughts?

jcsteh commented 1 year ago
  1. The reason the ms field is unlabelled is that it's effectively labelled by the radio button. It's also inside the Hold for: grouping. I can label it, but that also means we'll have a redundant label visually, since I can't assign non-visual labels in a cross-platform way. I guess that's better than the alternative though.
  2. I originally considered labelling the group "Hold level until:". However, this means that for the Disabled option, you get "Hold level until disabled", suggesting that the level is held until the watcher is disabled, which is precisely the opposite of what it does.
jcsteh commented 1 year ago

Btw, I'm curious as to why the unlabelled field is less obvious with VO navigation, given that it's right next to the ms radio button?

ScottChesworth commented 1 year ago

I'm curious as to why the unlabelled field is less obvious with VO navigation, given that it's right next to the ms radio button?

It's not. Here's a chronological list of how VO reports controls in the dialog with settings at their default states, starting from where focus lands then moving with VO+RightArrow:

  • Follow when last-touched track changes (check box)
  • Level type (label)
  • Level type (popup button)
  • 1 (check box)
  • 2 (check box)
  • Notify automatically for channels (label)
  • Disabled (reported as a radio button, but behaves different from expected, see list of gotchas later in comment)
  • Hold level (label)
  • Until reset (radio button selected by default)
  • For: ms (radio button)
  • When level reaches (label)
  • 0.00 (contents selected, edit text)
  • Dimmed (text, this being the field that becomes active when "for: ms" radio button is checked, in which case the "dimmed" disappears and we just hear "text").

Now let's go back to the default landing position and navigate the tab order instead, this is also slightly different on Mac:

So here's what I'd perceive to be Mac specific gotchas, some worth poking at, some not:

  1. Labeling isn't reported when navigating via the tab order. That's a VO thing, not our lookout. IMO tightening up VO navigation should be what we focus on if we're gonna put any time into refining this.
  2. It's confusing that the "Level type" text label prepends the control, whereas all other labels come after controls with VO navigation. The prepended ordering as we have it on Windows and with "Level type" on Mac is a lot more intuitive. Is it possible for other labels to appear like that? To put it another way, can we ascertain why the controls don't show up in the same order as they do when navigating by object in Windows?
  3. ON Windows, the "When level reaches" edit field appears before the group of radio buttons. On Mac, it's after with both navigation approaches, thus separating the ms field when that's visible/available to be populated.
  4. With VO, although radio buttons are reported as radio buttons, they aren't navigable like in native dialogs. Our radio buttons appear as individual items in the tab order and aren't selectable with unodified arrows. Contrast this against how the radio buttons appear on the General tab of the Privacy and Security pane in System Prefs. Is this a Swell issue perhaps?
  5. If the only hang up on adopting the language suggested in my prior comment is the "Disabled" radio button, would you be open to axing the Disabled setting? Seems like that could happen without losing functionality, a hold time of 0ms would do the same thing right?

I know this is long and I'm not expecting all of it to be possible, just figured it would be useful to have the current Mac UX thoroughly documented.

jcsteh commented 1 year ago
2. It's confusing that the "Level type" text label prepends the control, whereas all other labels come after controls with VO navigation. The prepended ordering as we have it on Windows and with "Level type" on Mac is a lot more intuitive. Is it possible for other labels to appear like that? To put it another way, can we ascertain why the controls don't show up in the same order as they do when navigating by object in Windows?

I have no idea. The order in Windows is the order in which they're specified in the code. SWELL (or Mac or VoiceOver) must be reordering them, but I don't know why. That's really screwed up.

4. With VO, although radio buttons are reported as radio buttons, they aren't navigable like in native dialogs. Our radio buttons appear as individual items in the tab order and aren't selectable with unodified arrows. Contrast this against how the radio buttons appear on the General tab of the Privacy and Security pane in System Prefs. Is this a Swell issue perhaps?

I'd say so. I certainly can't control it with what's available to me.

5. If the only hang up on adopting the language suggested in my prior comment is the "Disabled" radio button, would you be open to axing the Disabled setting?

That's how things originally were, but the decision to have these radio buttons (including Disabled) was made very intentionally due to user feedback. See #63. So, I don't think I'm open to this unless you can convince me that the arguments in #63 no longer hold. :)

ScottChesworth commented 1 year ago

I forgot the move to radio buttons was motivated by user confusion. Neither of us want more of that, so I guess we're keeping the Disabled option. Hmm, maybe I have another idea. What does Disabled do exactly? Does it fall back on one of REAPER's metering related settings in the Track Control Panels category of Prefs, or does it opperate independently and really not hold peaks at all?

jcsteh commented 1 year ago

OSARA implements holding levels itself using a timer. Disabled... disables that. :) REAPER metering isn't involved here (beyond being a source of raw data).

ScottChesworth commented 1 year ago

Gotcha. I think the label ordering is probably a bigger UX issue than the language, do you agree? If so, maybe it's worth getting Justin to have a look at whether anything SWELL is doing can be tightened up. I could send him short reproduction steps to illustrate the differences. If he wants to see code, where should I link to?

jcsteh commented 1 year ago

The fact that the unlabelled ms field causes problems for JAWS isn't great, and it seems it isn't great for VO either. So, I do think we should probably fix that.

As for the ordering, it's probably worth asking Cockos if they can tighten that up, yeah. Here's the code in question.

Before you do that, though, it'd be worth double checking with a custom Mac build whether our translation code is interfering somehow. To do that, just comment out the single call to translateDialog in peakWatcher.cpp and rebuild. It really shouldn't interfere, but that code is a bit obscure, so I want to be sure.

ScottChesworth commented 1 year ago

Oki doke. I don't have a local Mac environment here at the moment, will have to comment that line and send it as a PR for AppVeyor to build.

ScottChesworth commented 1 year ago

Holy lack of standardization Batman. I've been experimenting now I know where to find the code (muhahahaha).

  1. JAWS ignores our GROUPBOX labels in the tab order and doesn't read the title bar when a watcher dialog is open. It also super helpfully steps in to change initial selection in the Peak Watcher context menu, because convenience is King (unless you find it convenient to hear some context in your tab orders, that is).
  2. Narrator ignores GROUPBOX labels entirely, no matter whether you're tabbing or navigating objects. It manages to read the title bar though. Gee, thanks Narrator.

So that's 4 screen readers, 4 ridiculously different takes on the same code. Who programs these tools, and what do they have against consistent UX? I'd install Supernova and ZDSR to try those, but I'm not due a full mental breakdown until the weekend.

ScottChesworth commented 1 year ago

Anyway, it looks like the ms edit field issue is easy to fix. @jcsteh, I'm seeing 4 numbers after each control, coordinates of some kind. Can you clue me in on what each of them is?

jcsteh commented 1 year ago

I would have thought JAWS would read the group box label. It certainly read those group boxes years ago. I wonder if it has some heuristic for visually determining where the label is and we're not quite meeting that heuristic.

In order, the coordinates are left, top, width, height. Note that the order of the controls in the file is the order in which they will be created; the logical or semantic order. However, you could choose to place them in completely different locations visually such that the visual order is different. In our case, the logical and visual orders match. By default, MSAA (and UIA) use the previous static text as the label for a control. JAWS appears to have its own heuristics for an unlabelled control.

ScottChesworth commented 1 year ago

I was surprised Narrator ignored those too, thought object navigation was object navigation.

jcsteh commented 1 year ago

Win32 group boxes are weird. They aren't properly hierarchical, so NVDA (and I thought JAWS) has to do some weird acrobatics to make them seem hierarchical. I would have thought UIA (which is what Narrator solely uses) implemented similar acrobatics, but apparently not.

ScottChesworth commented 1 year ago

Justin has confirmed the group boxes appearing in a different order is a swell thing. I'm gonna make a case for it to be fixed at the source, but in case that turns out to be too much work, I've also come up with alternative naming that would work without us using group boxes. @jcsteh, do you have any objections to that approach if I can't get a swell fix? I'd get eyes on the case here to make sure we still have a GUI that's functional visually before rolling out any changes.

jcsteh commented 1 year ago

I could live with that. I'd like to review any PR for this before it gets merged, though.

ScottChesworth commented 1 year ago

Always wise when it's me submitting :)

ScottChesworth commented 1 year ago

Mac update: Justin says VoiceOver determines where group boxes will appear in its navigation automatically, more or less based on the center of the control. Cockos tend to position the center of their group boxes more or less halfway through the list of items it contains, so the issue that's appeared in our Peak Watcher GUI on Mac is scattered all throughout REAPER itself. It's impacting the UX wherever they use group boxes and also makes their tab controls show up in odd places in terms of VO navigation. Ideally, he'd like to find a way to override that detection heurstic and make VO navigation represent the actual order in the code Do we have anyone around who's worked with NSAccessibility? I'm wondering whether @vick08 or @mmulcahy222 might have some advice?

ScottChesworth commented 1 year ago

Potentially this also means that short-term, we could sort Peak Watcher out by redesigning the GUI. Sure. Easy. Urgh.

jcsteh commented 1 year ago

I would have thought that group boxes should be a parent of the controls they contain in the Mac accessibility tree. That's not the case in Win32, which SWELL is based on, but it may be the correct fix here; i.e. to ensure the controls are children of their group, not siblings. Though it looks like SWELL might be rendering groups as labels, so that might need to be fixed too.

Do you have to interact with groups in other Mac apps? If so, that would seem to confirm my findings here.

If we didn't want to mess with groups, perhaps the order of accessibility children could be overridden using the setAccessibilityChildren method in the new NSAccessibility protocol? That protocol is only supported on MacOS 10.10+, but there'd probably be a way to pull this off with the older framework too; I'd have to dig a bit.

ScottChesworth commented 1 year ago

Do you have to interact with groups in other Mac apps?

Typically yes. I haven't seen that anywhere in a REAPER window though. Think you're right that SWELL renders these as labels at the moment.

jcsteh commented 1 year ago

Here's the children attribute in the old NSAccessibility informal protocol if MacOS 10.10+ is a problem.

ScottChesworth commented 1 year ago

Thanks heaps for the research! I've passed both comments and links along.

jcsteh commented 1 year ago

Interestingly enough, I have some vague familiarity with some fairly obscure bits of MacOS accessibility having poked at the Mac accessibility code in Firefox. Where I struggle is how to override accessibility stuff in SWELL, since we don't have access to the Windows accessibility annotation stuff, but nor do we have access to the underlying native Mac widgets SWELL creates so that we can attach accessibility code to them. I'm not even sure if it's possible for code outside of the widget to tweak that widget on Mac like it is in Windows.

jcsteh commented 3 months ago

As ugly as it is, we're going to try just explicitly labelling everything in the dialog and getting rid of the groups. This is why we can't have nice things.

ScottChesworth commented 3 months ago

@jcsteh, as discussed I got verification check that the groups in Peak Watcher appear as expected, visually they're the same as the example in Control Panel -> System -> Advanced, with a grey border around the names of groups. I've scoured the NVDA log in both windows and can't find anything weird about what we're doing, so yeah, I reckon they're probably gonna have to go. Will send a PR sometime soon.

ScottChesworth commented 3 months ago

Examining Peak Watcher GUI using the Accessibility Insights app on Windows, controls contained within our groups don't seem to be children. Think they should be, no?

jcsteh commented 3 months ago

In Win32 UI, the actual controls aren't children of their group box. Unfortunately, the MSAA proxies don't correct this. NVDA has code to handle this oddity and I thought JAWS did too, but apparently not.

I especially would have thought that UI Automation (being Microsoft's chosen saviour for the future of accessibility) would map this correctly, but it turns out it doesn't either. This even breaks for Narrator in Advanced System Settings, which doesn't detect the groups at all. So it seems NVDA is the only screen reader that copes with this silliness. Given that, I definitely think we should get rid of them.

It's funny because a lot of people talk about how native accessibility is better than web accessibility, yet basics like this just work as they're supposed to on the web without much effort at all:

<fieldset>
  <legend>Notify automatically for channels:</legend>
  <label><input type="checkbox" accesskey="1">1</label>
  <label><input type="checkbox" accesskey="2">2</label>
</fieldset>

Done, fully accessible.