Created attachment 191983 [details] Open the database file. Go to Tools → Relationships. Try to edit relation and press Help. Open the attached database. Go to "Tools" → "Relationships". Right mouse click on the line, which connect the 2 tables → Edit. A dialog for appears. Press "Help". LO sends the message to the Browser and Base crashes. But, when writing this bug, I detected: Only click somewhere else like writing down this content for the bug instead of pressing "Help": LO crashes immediately. By the way: Help-ID: dbaccess/ui/relationdialog/dialog-action_area1 couldn't be found, but this isn't the reason for this bug. Resetting the user profile doesn't help here. Bug appears with Version: 24.2.0.1 (X86_64) / LibreOffice Community Build ID: b4d45829793cddfe67b58a53f495528c75738d8a CPU threads: 6; OS: Linux 5.14; UI render: default; VCL: kf5 (cairo+xcb) Locale: de-DE (de_DE.UTF-8); UI: en-US Calc: threaded Bug doesn't appear with LO 7.4.7.2 (my working version for Base - too many bugs in LO 7.5 and 7.6 to create databases there). So it's a regression.
Bug also doesn't appear in Version: 7.6.4.1 (X86_64) / LibreOffice Community Build ID: e19e193f88cd6c0525a17fb7a176ed8e6a3e2aa1 CPU threads: 6; OS: Linux 5.14; UI render: default; VCL: kf5 (cairo+xcb) Locale: de-DE (de_DE.UTF-8); UI: de-DE Calc: threaded So a new bug introduced with LO 24.2.0.1
I can't reproduce it in Version: 24.8.0.0.alpha0+ (X86_64) / LibreOffice Community Build ID: 877014b0b7050ba3fce1c0126279125640117313 CPU threads: 8; OS: Linux 6.1; UI render: default; VCL: x11 Locale: es-ES (es_ES.UTF-8); UI: en-US Calc: threaded
(In reply to Xisco Faulí from comment #2) > I can't reproduce it in > > Version: 24.8.0.0.alpha0+ (X86_64) / LibreOffice Community > Build ID: 877014b0b7050ba3fce1c0126279125640117313 > CPU threads: 8; OS: Linux 6.1; UI render: default; VCL: x11 > Locale: es-ES (es_ES.UTF-8); UI: en-US > Calc: threaded Can't reproduce it with SAL_USE_VCLPLUGIN=gen ./soffice and the same LO 24.2.0.1-version So it seems to be another bug for VCL: kf5. Will try with gtk.
Tested on another system. Same buggy behavior with KDE, no buggy behavior with Gnome. So a special KDE-bug.
Created attachment 191996 [details] gdb bt On pc Debian x86-64 with master sources updated today (+kf5 rendering but not with gtk3), I could reproduce this.
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?
(In reply to Julien Nabet from comment #6) > Does it seem OK to you? FWIW, the macOS implementation uses the same approach as your patch in osx/documentfocuslistener.cxx.
(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.)
(In reply to Michael Weghorn from comment #8) > (In reply to Julien Nabet from comment #6) > > Michael: with this patch, LO doesn't crash: > > ... > 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: > > ... > 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): > > ... Thank you a lot for the very detail response! I've just submitted https://gerrit.libreoffice.org/c/core/+/162182 on gerrit following your guidelines.
Julien Nabet committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/1a637a07a0fb23f4d4bfac69378caff7ee965737 tdf#159213: fix Base crash when choosing "Help" in relations design (kf5) It will be available in 24.8.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Patch cherry-picked on 24.2 by Michael (thank you for this! :-)) and waiting for review here: https://gerrit.libreoffice.org/c/core/+/162141
Thanks for the fix, Julien! (In reply to Robert Großkopf from comment #0) > Bug doesn't appear with LO 7.4.7.2 (my working version for Base - too many > bugs in LO 7.5 and 7.6 to create databases there). So it's a regression. This most likely started crashing with the following commit that enabled handling for the corresponding a11y CHILD event, i.e. the underlying issue was there before, but only started to cause a crash once more accessibility handling was implemented for the Qt-based VCL plugins. commit be8b031d8b3c66b223ea2478f1129427f3a362bd Author: Michael Weghorn Date: Thu Sep 7 09:51:38 2023 +0200 qt a11y: Forward CHILD event for removed child No longer comment out the code to send a `QAccessible::ObjectDestroyed` event when receiving an `AccessibleEventId::CHILD` event with its `OldValue` set. The underlying issues causing crashes previously seem to be fixed, I can no longer reproduce these on Debian testing with Orca 44.1-2 when using the font color popup as described in commit 734d10ed3612d75edcee145475ddd0b0165efeac Author: Michael Weghorn <m.weghorn@posteo.de> Date: Fri Apr 14 16:57:09 2023 +0300 qt a11y: Send QAccessible::ObjectCreated event for correct object > Adapting this for the case where a child has > been removed (bridged to Qt as `QAccessible::ObjectDestroyed` > event) would currently results in crashes when closing the > application e.g. after using the character font color popup in > the Writer toolbar. This needs further investigation, so don't > send the event for now, but add a `SAL_WARN`. Change-Id: Ib8f21850dd56645cf64a74be0e1ff8242615b928 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/156647 Tested-by: Jenkins Reviewed-by: Michael Weghorn (In reply to Michael Weghorn from comment #8) > 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.) I've added that to my task list and will come back to it as I find time.
Julien Nabet committed a patch related to this issue. It has been pushed to "libreoffice-24-2": https://git.libreoffice.org/core/commit/2797c7f803792f3539fc86ac5af06da23ec7cc2e tdf#159213: fix Base crash when choosing "Help" in relations design (kf5) It will be available in 24.2.1. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Julien Nabet committed a patch related to this issue. It has been pushed to "libreoffice-24-2-0": https://git.libreoffice.org/core/commit/e5a0d2f48893af6480f5539208ee202ee536a033 tdf#159213: fix Base crash when choosing "Help" in relations design (kf5) It will be available in 24.2.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
(In reply to Michael Weghorn from comment #8) > 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.) https://gerrit.libreoffice.org/c/core/+/163427 addresses one more place that looks a bit suspicious. I'd still leave the workaround in the Qt-specific code in place for a bit longer for now to see whether anything else shows up.
Michael Weghorn committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/694670e13143f41e211dd0e183516d08008dd12b tdf#159213 a11y: Only send child event if there's a child It will be available in 24.8.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.