https://bugs.documentfoundation.org/show_bug.cgi?id=159213
--- Comment #8 from Michael Weghorn <[email protected]> --- (In reply to Julien Nabet from comment #6) > Michael: with this patch, LO doesn't crash: > diff --git a/vcl/qt5/QtAccessibleEventListener.cxx > b/vcl/qt5/QtAccessibleEventListener.cxx > index d6a404e6947e..ff813e6bcbfd 100644 > --- a/vcl/qt5/QtAccessibleEventListener.cxx > +++ b/vcl/qt5/QtAccessibleEventListener.cxx > @@ -211,13 +211,13 @@ void QtAccessibleEventListener::notifyEvent(const > css::accessibility::Accessible > case AccessibleEventId::CHILD: > { > Reference<XAccessible> xChild; > - if (aEvent.NewValue >>= xChild) > + if ((aEvent.NewValue >>= xChild) && xChild.is()) > { > QAccessible::updateAccessibility(new QAccessibleEvent( > QtAccessibleRegistry::getQObject(xChild), > QAccessible::ObjectCreated)); > return; > } > - if (aEvent.OldValue >>= xChild) > + if ((aEvent.OldValue >>= xChild) && xChild.is()) > { > QAccessible::updateAccessibility(new QAccessibleEvent( > QtAccessibleRegistry::getQObject(xChild), > QAccessible::ObjectDestroyed)); > > Does it seem OK to you? Generally looks reasonable to me, though I think that a `CHILD` event with no valid accessible set is invalid by itself and shouldn't be emitted in the first place. Fixing the place where the invalid event is emitted is IMHO worth it. Looking at the backtrace, presumably this for the case here: diff --git a/svtools/source/brwbox/editbrowsebox.cxx b/svtools/source/brwbox/editbrowsebox.cxx index 927e203dd14a..515c102379b3 100644 --- a/svtools/source/brwbox/editbrowsebox.cxx +++ b/svtools/source/brwbox/editbrowsebox.cxx @@ -967,7 +967,7 @@ namespace svt if (!IsEditing()) return; - if ( isAccessibleAlive() ) + if (isAccessibleAlive() && m_aImpl->m_xActiveCell) { commitBrowseBoxEvent( CHILD, Any(), Any( m_aImpl->m_xActiveCell ) ); m_aImpl->clearActiveCell(); In theory, the suggested check in QtAccessibleEventListener::notifyEvent should IMHO not be needed, but there might of course be other places sending such events. A safe way might be to add an assert there (so issues are identified in dev builds) + the check as a workaround (so release builds don't crash), like this (and same for the other case): --- a/vcl/qt5/QtAccessibleEventListener.cxx +++ b/vcl/qt5/QtAccessibleEventListener.cxx @@ -213,6 +213,10 @@ void QtAccessibleEventListener::notifyEvent(const css::accessibility::Accessible Reference<XAccessible> xChild; if (aEvent.NewValue >>= xChild) { + assert(xChild.is() && "AccessibleEventId::CHILD event without valid child set"); + // tdf#159213 for now, workaround invalid events being sent and don't crash in release builds + if (!xChild.is()) + return; QAccessible::updateAccessibility(new QAccessibleEvent( QtAccessibleRegistry::getQObject(xChild), QAccessible::ObjectCreated)); return; This could then be backported to release branches. And a possible consideration might then be to revise + fix all of the places where a CHILD event is sent, and drop the workaround (just on master) again, once all of these should be safe. (I'm happy to do the latter as I find time if it makes sense.) -- You are receiving this mail because: You are the assignee for the bug.
