wxWidgets / Phoenix

wxPython's Project Phoenix. A new implementation of wxPython, better, stronger, faster than he was before.
http://wxpython.org/
2.21k stars 509 forks source link

add CreateAccessible and GetOrCreateAccessible #2515

Open DietmarSchwertberger opened 4 months ago

DietmarSchwertberger commented 4 months ago

This PR improves accessibility support on Windows, once ext/wxwidgets is updated to a version after January 11. ( after commit https://github.com/wxWidgets/wxWidgets/commit/bd899c667129a13e265335638a9ddd81e3f8b75e "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 commented 4 months ago

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

swt2c commented 4 months ago

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

DietmarSchwertberger commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 https://github.com/wxWidgets/wxWidgets/commit/bd899c667129a13e265335638a9ddd81e3f8b75e

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 (https://github.com/wxGlade/wxGlade/issues/534#issuecomment-1812685781). 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 commented 4 months ago

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