wxRichTextCtrl is inaccessible to screen readers (Issue #26051)
Ethin Probst
ethindp created an issue (wxWidgets/wxWidgets#26051)Description
Bug description
wxRichTextCtrl is inaccessible to assistive technology. When using NVDA, using object navigation causes it to just say "text" when moving over the control.
Expected vs observed behaviour
wxRichTextCtrl should be fully accessible. However, it neither appeared in the tab order nor did it indicate that it was even an edit control to begin with.
To Reproduce
- Install NVDA.
- Open an app that uses
wxRichTextCtrl. - Use object navigation to navigate the accessibility tree. Navigate to where the
wxRichTextCtrlshould be. - NVDA will just say "text" and nothing else.
Platform and version information
- wxWidgets version you use: 3.3.1 and master
- wxWidgets port you use: wxMSW (but may occur on others)
- OS and its version: Windows 10
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26051)Unfortunately wxRichTextCtrl is not maintained by its original author since a very long time any more, so any improvements to it depend on people submitting patches/PRs making them. If you can add a11y support to it, please consider doing it.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Joshua Lee Ockert
torstenvl left a comment (wxWidgets/wxWidgets#26051)I'm interested in taking a look at this, but it will take a fair amount of experimentation and exploration. No promises.
Starting to get a shape of the problem, I note that most screen readers allow you to scroll line by line through long text. However, wxRichTextCtrl doesn't allow keyboard mouse navigation when it's read-only. If we want to allow using accessibility features that way, we'll have to modify that.
(Also, the documentation says that the read-only style is wxRE_READONLY, but all of the source uses wxTE_READONLY -- this works because both are defined as the same value, but it makes mentally mapping out execution paths more difficult when none of the code references the actual name of the constant. Should probably fix that.)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26051)I'm interested in taking a look at this, but it will take a fair amount of experimentation and exploration. No promises.
All I can say that any improvements here would be welcome, so if you manage to do it — great!
Starting to get a shape of the problem, I note that most screen readers allow you to scroll line by line through long text. However, wxRichTextCtrl doesn't allow keyboard mouse navigation when it's read-only. If we want to allow using accessibility features that way, we'll have to modify that.
This is weird and I didn't even know it. But I really don't know much/anything about this code, unfortunately.
(Also, the documentation says that the read-only style is wxRE_READONLY, but all of the source uses wxTE_READONLY -- this works because both are defined as the same value, but it makes mentally mapping out execution paths more difficult when none of the code references the actual name of the constant. Should probably fix that.)
Yes, this is weird too, we should probably at least use #define wxRE_READONLY wxTE_READONLY (and same for MULTILINE).
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Joshua Lee Ockert
torstenvl left a comment (wxWidgets/wxWidgets#26051)I've got good news and bad news.
The good news is that, once I figured out how the pieces fit together, implementing wxAccessible for wxRichTextCtrl was incredibly simple (see below).
The bad news is that wxAccessible only implements legacy IAccessible, which is not nearly fully-featured enough for text editing. For interactive accessibility in custom-drawn text editing contexts, we need to implement IAccessibleText, which is part of the newer IAccessibility2 system. The best we can get with wxAccessible/IAccessible is to make the full text available all at once to NVDA. It doesn't expose cursor position, current line, etc., so screen readers can't respond to interaction with the control, truly blind users won't be able to edit, etc.
So... a combined architectural/RELENG question for @vadz:
- Would you like me to try to implement only for
wxRichTextCtrl(and potentially later forwxStyledTextCtrl)? If so, I think I might be able to get it done for 3.4.0. (when is feature freeze?). This is a bit hacky, but it should have the effect of not pollutingwxAccessiblewith extras that are unnecessary for most components. - Or would you like me to try to update
wxAccessibleto implementIAccessible2across the board? If so, I doubt I can get this done in time for 3.4.0. This is probably the "right" answer in terms of clean architecture, but it is really only needed to give usIAccessibleTextfor custom-drawn text editing controls, so it's a fair amount of extra code in every accessible build for something that only benefits two components that aren't in core.
Note: macOS does not use the IAccessible / IAccessible2 paradigm. Eventually, making wxAccessible implement the minimums required for NSAccessibility may necessitate doing most of the groundwork for IAccessible2 anyway. But I'm not sure that's a realistic goal for 3.4.0. On the other hand, if we want to do this later in 3.4.x, it might be worth starting the work now, in case it turns out that it involves API changes (complete unknown at this time).
Minimal legacy IAccessible implementation is:
--- a/include/wx/richtext/richtextctrl.h
+++ b/include/wx/richtext/richtextctrl.h
@@ -2303,6 +2303,10 @@ protected:
#endif
#endif // !__WXUNIVERSAL__
+#if wxUSE_ACCESSIBILITY
+ virtual wxAccessible* CreateAccessible() override;
+#endif // wxUSE_ACCESSIBILITY
+
and
--- a/src/richtext/richtextctrl.cpp
+++ b/src/richtext/richtextctrl.cpp
@@ -5583,5 +5583,69 @@ int wxRichTextContextMenuPropertiesInfo::AddItems(wxRichTextCtrl* ctrl, wxRichTe
return GetCount();
}
+#if wxUSE_ACCESSIBILITY
+
+#include "wx/access.h"
+
+class wxRichTextCtrlAccessible : public wxWindowAccessible
+{
+public:
+ explicit wxRichTextCtrlAccessible(wxRichTextCtrl* win)
+ : wxWindowAccessible(win)
+ {
+ }
+
+ wxAccStatus GetName(int childId, wxString* name) override
+ {
+ if ( childId != wxACC_SELF )
+ return wxWindowAccessible::GetName(childId, name);
+
+ *name = "Rich Text Control";
+ return wxACC_OK;
+ }
+
+ wxAccStatus GetRole(int childId, wxAccRole* role) override
+ {
+ if ( childId != wxACC_SELF )
+ return wxWindowAccessible::GetRole(childId, role);
+
+ *role = wxROLE_SYSTEM_TEXT;
+ return wxACC_OK;
+ }
+
+ wxAccStatus GetState(int childId, long* state) override
+ {
+ if ( childId != wxACC_SELF )
+ return wxWindowAccessible::GetState(childId, state);
+
+ wxRichTextCtrl* ctrl = wxStaticCast(GetWindow(), wxRichTextCtrl);
+
+ *state = wxACC_STATE_SYSTEM_FOCUSABLE;
+ if ( ctrl->HasFocus() )
+ *state |= wxACC_STATE_SYSTEM_FOCUSED;
+ if ( !ctrl->IsEditable() )
+ *state |= wxACC_STATE_SYSTEM_READONLY;
+
+ return wxACC_OK;
+ }
+
+ wxAccStatus GetValue(int childId, wxString* strValue) override
+ {
+ if ( childId != wxACC_SELF )
+ return wxWindowAccessible::GetValue(childId, strValue);
+
+ wxRichTextCtrl* ctrl = wxStaticCast(GetWindow(), wxRichTextCtrl);
+ *strValue = ctrl->GetValue();
+ return wxACC_OK;
+ }
+};
+
+wxAccessible* wxRichTextCtrl::CreateAccessible()
+{
+ return new wxRichTextCtrlAccessible(this);
+}
+
+#endif // wxUSE_ACCESSIBILITY
+
#endif
// wxUSE_RICHTEXT
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26051)Thanks for looking into this, I agree that having just IAccessible implementation is not incredibly useful, but it's probably still better than nothing, so if you'd like to make a PR adding it, I'd be glad to merge it.
Concerning IAccessible2, I think it might be useful to try implementing it for wxRTC first and then check what needs to be generalized in order to apply it to wxSTC and, possibly, other text-like controls. I suggest starting with the code in include/wx/private/accessibletext.h or something like this, to be able to reuse it from other places later even if you only implement it for wxRTC initially. Or maybe put this file in include/wx/msw/private if it's going to be wxMSW-specific.
Thanks again!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Ethin Probst
ethindp left a comment (wxWidgets/wxWidgets#26051)@vadz @torstenvl You should probably just jump straight to implementing UIA, honestly, and skipping IA/IA2. UIA I believe offers everything you would ever need for the vast super-majority of controls.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Joshua Lee Ockert
torstenvl left a comment (wxWidgets/wxWidgets#26051)wxWidgets still targets Windows XP support. From what I recall, Windows XP doesn't support UIA until SP3, if I understand correctly. That said, I have to imagine that the Venn diagram intersection of people who are using Windows XP; use screen readers; haven't updated to SP3; and use software built with wxWidgets... has to be vanishingly small.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26051)We don't support XP in master, where all these changes would go and I think we could drop support for 7 when 3.4 is released, so this shouldn't be a problem.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.