Skip to content

add CreateAccessible and GetOrCreateAccessible#2515

Closed
DietmarSchwertberger wants to merge 2 commits intowxWidgets:masterfrom
DietmarSchwertberger:ACCESSIBILITY
Closed

add CreateAccessible and GetOrCreateAccessible#2515
DietmarSchwertberger wants to merge 2 commits intowxWidgets:masterfrom
DietmarSchwertberger:ACCESSIBILITY

Conversation

@DietmarSchwertberger
Copy link
Copy Markdown
Contributor

This PR improves accessibility support on Windows, once ext/wxwidgets is updated to a version after January 11.
( after commit wxWidgets/wxWidgets@bd899c6 "Add missing documentation of wxWindow accessibility functions" )

This PR adds support for CreateAccessible, allowing on-demand creation of custom accessibility support.
(At the moment, if a widget needs customized support, a wx.Accessible instance needs to be assigned in advance to a widget using SetAccessible.)

The PR is not complete yet.
I'm posting it already, because I need help.
The remaining problem is that CreateAccessible in a derived class needs to keep a reference to its return value.

E.g. this code will fail with an address exception if self._acc = ret = MyAcc(self) is replaced with ret = MyAcc(self)

class MyAcc(wx.Accessible): # you must be on Windows to test this
    def GetName(self, childid):
        return (wx.ACC_OK, 'My Name')
    def GetDescription(self, childid):
        return (wx.ACC_OK, 'My Description')
    def GetValue(self, childid):
        return (wx.ACC_OK, 'My Value')

class MyAccessibleTextCtrl(wx.TextCtrl):
    def CreateAccessible(self):
        # which each window can override to create a new wxAccessible-derived object.
        self._acc = ret = MyAcc(self)  # reference needs to be kept
        return ret

I tried adding a SIP annotation /KeepReference/ manually to sip/gen/window.sip:

    wxAccessible * CreateAccessible() /KeepReference/ ;

This did not help, though. The code that actually keeps the reference is not called.

@DietmarSchwertberger
Copy link
Copy Markdown
Contributor Author

@swt2c : How familiar are you with SIP? Probably it would be best to ask on the PyQt/SIP mailing list for support, right?

@swt2c
Copy link
Copy Markdown
Collaborator

swt2c commented Jan 15, 2024

Hey Dietmar, I'm fairly familiar with sip, give me a little time to look at it...

@DietmarSchwertberger
Copy link
Copy Markdown
Contributor Author

DietmarSchwertberger commented Jan 15, 2024

OK. I think you probably don't want to build a version and try it. It's only for Windows and requires a screen reader like NVDA or a tool like inspect.exe to be installed to trigger calls to the accessibility methods via OLE / COM. Also, experience is that with 64 bit builds there are often problems and accessibility does not work at all.
Additionally, while mixed mode debugging is a great feature, Visual Studio crashes a lot when accessibility tools are running.

So, here is some code.
Keep in mind that CreateAccessible is a virtual method. It's typically called from the method GetOrCreateAccessible.

When I manually add /KeepReference/ to window.sip and textctrl.sip then methods meth_wx..._CreateAccessible are generated:
(with sipKeepReference right before sipResObj is returned)

PyDoc_STRVAR(doc_wxTextCtrl_CreateAccessible, "CreateAccessible(self) -> typing.Optional[Accessible]");

extern "C" {static PyObject *meth_wxTextCtrl_CreateAccessible(PyObject *, PyObject *);}
static PyObject *meth_wxTextCtrl_CreateAccessible(PyObject *sipSelf, PyObject *sipArgs)
{
    PyObject *sipParseErr = SIP_NULLPTR;
    bool sipSelfWasArg = (!sipSelf || sipIsDerivedClass((sipSimpleWrapper *)sipSelf));

    {
         ::wxTextCtrl *sipCpp;

        if (sipParseArgs(&sipParseErr, sipArgs, "B", &sipSelf, sipType_wxTextCtrl, &sipCpp))
        {
             ::wxAccessible*sipRes;
            PyObject *sipResObj;

            PyErr_Clear();

            Py_BEGIN_ALLOW_THREADS
            sipRes = (sipSelfWasArg ? sipCpp-> ::wxTextCtrl::CreateAccessible() : sipCpp->CreateAccessible());
            Py_END_ALLOW_THREADS

            if (PyErr_Occurred())
                return 0;

            sipResObj = sipConvertFromType(sipRes,sipType_wxAccessible,SIP_NULLPTR);
            sipKeepReference(sipSelf, -20, sipResObj);

            return sipResObj;
            
        }
    }

    sipNoMethod(sipParseErr, sipName_TextCtrl, sipName_CreateAccessible, doc_wxTextCtrl_CreateAccessible);

    return SIP_NULLPTR;
}

But actually when CreateAccessible of my Python class is called, then this is not via meth_wxTextCtrl_CreateAccessible , but through this function in sip/cpp/sip_coremodule.cpp:

 ::wxAccessible* sipVH__core_129(sip_gilstate_t sipGILState, sipVirtErrorHandlerFunc sipErrorHandler, sipSimpleWrapper *sipPySelf, PyObject *sipMethod)
{
     ::wxAccessible* sipRes = 0;
    PyObject *sipResObj = sipCallMethod(SIP_NULLPTR, sipMethod, "");

    sipParseResultEx(sipGILState, sipErrorHandler, sipPySelf, sipMethod, sipResObj, "H0", sipType_wxAccessible, &sipRes);
    return sipRes;
}

I'm not good at C++ or wxWidgets. I tried inserting a sipKeepReference(&sipPySelf->ob_base, -20, sipResObj);, but that did not work and adding Py_XINCREF(sipResObj) did not help.

This is the call stack when I have a breakpoint at my Python method CreateAccessible():
I can see sipwxTextCtrl::CreateAccessible() and sipVH__core_129, but not the method meth_wxTextCtrl_CreateAccessible that would keep a reference:
A breakpoint there is never hit.
grafik

When my Python class does not keep a reference to the return value itself, then I get an exception after CreateAccessible returns:
grafik

The exception is in in window.cpp, right after the call to GetOrCreateAccessible() which in turn called CreateAccessible():

grafik

The stack backtrace at the exception is not interesting, but the two screenshots below, showing the locals, are:
grafik

I don't know how to interpret m_accessible in the locals window at window.cpp level:
When I don't keep a reference, the sipWxAccessible is not there. (Compare to the next screenshot.)
grafik

When my Python class keeps a reference, then it looks slightly different - it has a sipWxAccessible:
grafik

@swt2c
Copy link
Copy Markdown
Collaborator

swt2c commented Jan 17, 2024

Thanks for the detailed debugging. :-) After looking at it, I think CreateAccessible() should be marked with /Factory/ and not /KeepReference/. Do you mind giving that a try?

@DietmarSchwertberger
Copy link
Copy Markdown
Contributor Author

Ah, thanks a lot. That seems to work. From the documentation I would never have guessed that...

From debugging, the methods like meth_wxTextCtrl_CreateAccessible are still not called, but the change in sipVH__core_129 makes the difference:
There is now the sipParseResultEx argument "H2" instead of "H0".
"2" matches this flag:
#define FMT_RP_FACTORY 0x02 /* /Factory/ or /TransferBack/. */

 ::wxAccessible* sipVH__core_129(sip_gilstate_t sipGILState, sipVirtErrorHandlerFunc sipErrorHandler, sipSimpleWrapper *sipPySelf, PyObject *sipMethod)
{
     ::wxAccessible* sipRes = 0;
    PyObject *sipResObj = sipCallMethod(SIP_NULLPTR, sipMethod, "");

    sipParseResultEx(sipGILState, sipErrorHandler, sipPySelf, sipMethod, sipResObj, "H2", sipType_wxAccessible, &sipRes);

    return sipRes;
}

I have pushed the modification. The checks will continue to fail until wxWidgets points to a more recent version, after commit wxWidgets/wxWidgets@bd899c6

Should I leave the status at "Draft" for the time, or switch it already to "Ready for review"?

I will now continue to work on wxGrid accessibility support (wxGlade/wxGlade#534 (comment)). That's why I looked into this topic.
At some time, I will also update Discuss (https://discuss.wxpython.org/t/has-wx-accessible-ever-worked/35454).

@RobinD42
Copy link
Copy Markdown
Member

This pull request has been mentioned on Discuss wxPython. There might be relevant details there:

https://discuss.wxpython.org/t/has-wx-accessible-ever-worked/35454/7

@DietmarSchwertberger DietmarSchwertberger changed the title add CreateAccessible and GetOrCreateAcceessible add CreateAccessible and GetOrCreateAccessible Apr 12, 2024
@jmoraleda
Copy link
Copy Markdown
Contributor

jmoraleda commented May 17, 2024

I just saw this, after I created this PR to update wxWidgets. In that PR, I added the minimal support for accessibility required to compile in non-MSW platforms mimicking for the new methods GetOrCreateAccessible and CreateAccessible what we do now for GetAccessible.

@swt2c
Copy link
Copy Markdown
Collaborator

swt2c commented May 18, 2024

Hey @DietmarSchwertberger, the master branch now has moved to v3.2.5 and we also included part of your changes in this PR. The changes for tweaker_tools were not included because they don't compile on GTK and macOS.

@DietmarSchwertberger
Copy link
Copy Markdown
Contributor Author

Thanks a lot.
I did not build and test the version without the tweaker tools modification, but I think that we need them to make this actually work in Python.
As jmoraleda suggested, they could be applied only if #if wxUSE_ACCESSIBILITY. I don't see a way how tweaker_tools can access such information, though. A dirty hack would be to check inside removeVirtuals whether CreateAccessible was there.

I modeled the changes in etg/windows.py after the existing code which is commented with wxAccessbile is MSW only. Provide a NotImplemented fallback for the other platforms.
This seems a bit over-engineering. A simple flag or no-op methods would probably have been easier for the user.

@DietmarSchwertberger
Copy link
Copy Markdown
Contributor Author

I have modified and built 4.2.2 on Windows and I think this should do the trick to enable CreateAcessible:

--- a/etgtools/tweaker_tools.py
+++ b/etgtools/tweaker_tools.py
@@ -20,6 +20,7 @@ import textwrap
 
 
 PY3 = sys.version_info[0] == 3
+isWindows = sys.platform.startswith('win')
 
 magicMethods = {
     'operator!='    : '__ne__',
@@ -485,6 +486,9 @@ def addWindowVirtuals(klass):
         #void UpdateWindowUI(long flags = wxUPDATE_UI_NONE);
         #void DoUpdateWindowUI(wxUpdateUIEvent& event) ;
     ]
+    if isWindows:
+        # does not compile on GTK and macOS.
+        publicWindowVirtuals.append( ('CreateAccessible', 'wxAccessible* CreateAccessible()') )
 
     protectedWindowVirtuals = [
         ('ProcessEvent',              'bool ProcessEvent(wxEvent & event)'),

@swt2c : Would you mind adding these lines to tweaker_tools.py? I would then close this PR.

@DietmarSchwertberger
Copy link
Copy Markdown
Contributor Author

see PR #2618 for the required change to etg_tools.py

@RobinD42
Copy link
Copy Markdown
Member

This pull request has been mentioned on Discuss wxPython. There might be relevant details there:

https://discuss.wxpython.org/t/cant-build-wxpython-4-2-2-on-some-suse-distributions/40176/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants