Created attachment 161562 [details] screencast This is a follow-up of bug 129428 and bug 129587 Steps to reproduce: 1. Open https://bugs.documentfoundation.org/attachment.cgi?id=156720 2. double click chart 3. select Y error bars 4. Format Y error bars 5. Unselect "Same value for both" or change any other property -> bug 129587 crashes here. 6. Click Ok 7. Select any other object ( e.g. the wall ) 8. Change any property -> Crashes Version: 7.0.0.0.beta1+ Build ID: 2506d8221dd940dfd93d3d7c183430ba6ba3089d CPU threads: 4; OS: Linux 4.19; UI render: default; VCL: x11 Locale: en-US (en_US.UTF-8); UI: en-US Calc: threaded
Regression introduced by: https://cgit.freedesktop.org/libreoffice/core/commit/?id=2d01ed9e8be543460e41e009fa992103a7c8d4c0 author Muhammet Kara <muhammet.kara@collabora.com> 2019-11-25 21:55:31 +0300 committer Muhammet Kara <muhammet.kara@collabora.com> 2019-11-26 15:39:56 +0100 commit 2d01ed9e8be543460e41e009fa992103a7c8d4c0 (patch) tree bd75130e564374647046f03ad4a334c1876d9dd0 parent 9b6b134b3e605236e90aca089867f08d659a6076 (diff) tdf#94288: Show chart props sidebar on activation Bisected with: bibisect-linux64-6.5 Adding Cc: to Muhammet Kara
@Michael, since you fixed bug 129428 and bug 129587, I thought you might be interested in this issue
Also reproducible with these steps: 1. Open attachment 149601 [details] from bug 123722 2. Double click on the chart 3. Click on any number on the X or Y axis 4. Right click on any of the data points - Data Range 5. Cancel the dialog -> Crash. terminate called after throwing an instance of 'com::sun::star::lang::IndexOutOfBoundsException'
Created attachment 161566 [details] bt with debug symbols (gen) On pc Debian x86-64 with master sources updated today + gen rendering, I got an assertion.
(gdb) p aCID $2 = "CID/D=0:CS=0:CT=0:Series=0"
For the record, I got the assertion too after having reverted locally Muhammet Kara's patch, so perhaps the assertion is unrelated.
Ah - I am interested of course =) but my fix was in fact a complete accident ;-) presumably it is a side-effect of lurking not actually destroying the sidebar panel widgets - but keeping them around: a vital optimization for mobile. Is there a stack-trace ?
(In reply to Michael Meeks from comment #7) > Ah - I am interested of course =) but my fix was in fact a complete accident > ;-) presumably it is a side-effect of lurking not actually destroying the > sidebar panel widgets - but keeping them around: a vital optimization for > mobile. > > Is there a stack-trace ? Don't know if it's related but https://bugs.documentfoundation.org/attachment.cgi?id=161566 contains a stacktrace with symbols.
*** Bug 134710 has been marked as a duplicate of this bug. ***
Created attachment 163734 [details] Call Stack I get a crash, when I select a line of a data series and then change the line corner style in the sidebar. The attached call stack is from my debug build of LO 7.1 I get the crash too in Version: 7.0.0.2 (x64) Build ID: c01aa64b6c3d89ebe5fe69c28c7adb24eb85249c CPU threads: 8; OS: Windows 10.0 Build 18362; UI render: Skia/Vulkan; VCL: win Locale: de-DE (en_US); UI: en-US Calc: CL with the error message window: LibreOffice 7.0 - Fatal Error /!\ -1
Created attachment 163784 [details] sample document
(In reply to Michael Meeks from comment #7) > Ah - I am interested of course =) but my fix was in fact a complete accident > ;-) presumably it is a side-effect of lurking not actually destroying the > sidebar panel widgets - but keeping them around: a vital optimization for > mobile. > > Is there a stack-trace ? I got the same kind of crash, with slightly different repro steps. I tried to understand what happened in the code, it seems that the ModifyListener of the different panels sometimes (always?) stay active once created, then they try to update the corresponding data, even if the currently selected object is not of the correct type. This causes issues with axes especially: either an assert in debug mode (complaining about the object type, the one in Comment 8) or an exception otherwise. I managed to avoid the crash I saw by adding a sanity check inside ChartAxisPanel::updateData(), but what would be the correct way to solve this? Just more checks on object types at each updateData? Or a way to remove listeners without destroying the widgets?
*** Bug 136611 has been marked as a duplicate of this bug. ***
I think we should put max priority here since: - it's a regression - it's a crash - these are not corner cases - it affect all envs
Hi Leyan, I'm interested in your sanity-check as a patch. It's curious that the chart sidebars are particularly problematic in this regard - then again - if there is no chart selected they should behave quiescently (I would hope): OUString getCID(const css::uno::Reference<css::frame::XModel>& xModel) { css::uno::Reference<css::frame::XController> xController(xModel->getCurrentController()); css::uno::Reference<css::view::XSelectionSupplier> xSelectionSupplier(xController, css::uno::UNO_QUERY); if (!xSelectionSupplier.is()) return OUString(); uno::Any aAny = xSelectionSupplier->getSelection(); assert(aAny.hasValue()); OUString aCID; aAny >>= aCID; #if defined DBG_UTIL && !defined NDEBUG ObjectType eType = ObjectIdentifier::getObjectType(aCID); assert(eType == OBJECTTYPE_AXIS); #endif return aCID; } could happily return an empty string instead of asserting in dbgutil mode - or (perhaps more helpfully) logging a warning instead of asserting.
Hi Michael, With the attached patch, I no longer get the crash I was getting initially. I am not sure whether the original bug reporter had the exact same problem, there may be other issues with other types of panels. Removing the assert you mentioned is the first step, but even without it, LO crashes when a ModifyListener calls ChartAxisPanel::updateData() while the currently selected object is not an axis. In the updateData, the getCID() function returns the CID of the selected object, but then ChartAxisPanel tries to use it through isLabelShown, getAxisForCID, getAxis, getMaximumAxisIndexByDimension and that's where an IndexOutOfBounds exception appears, because there is no dimension information in the CID.
Created attachment 165380 [details] patch preventing the crash
Patch looks plausible. I would SAL_WARN with a sensible domain if the object type is wrong. Otherwise the 'return 'looks fine when it's not an axis. I would avoid the redundant extra braces - that module has a consistent no-extra-brace-for-one-line statements style. Lets get it in =) thanks.
I put an updated version of the patch on gerrit: https://gerrit.libreoffice.org/c/core/+/102544 As it is the first time I go through this process, please tell me if something is wrong.
Please improve your commit message. A reference to this bug is written as tdf#133630. It will automatically be replaced with a clickable link. Do not use https://.. for this purpose. The first line should not say "A patch to solve the bug ...". The fact, that the patch will solve this bug is already given by the part "tdf#133630". But please describe what you have done to solve it. Keep in mind, that the first line should have <50 characters. You should have got a warning when you upload your patch to Gerrit.
(In reply to Leyan from comment #19) > I put an updated version of the patch on gerrit: > https://gerrit.libreoffice.org/c/core/+/102544 > > As it is the first time I go through this process, please tell me if > something is wrong. Hi Leyan, Thank you very much for you patch. I applied it locally and the steps from the description still make LibreOffice to crash ( same steps as in the screencast ). Could you please explain which steps your patch fix ?
Created attachment 165642 [details] updated patch checking object type for the different side panels Hi Xisco, Sorry, I did not read the bug report carefully enough the first time. I came here after trying to solve a crash I got on my own, which causes the same message as the backtrace given in comment 4, so I thought it was the same. What I got was related to axes (for example open a chart with a secondary axis, and alternate between editing axes and chart series properties). So my patch only solved this case. I reproduced the crash using your steps, it is indeed the same reason, but with error bars and chart series. The same kind of object type check can be used and should avoid the crash you get, please try the updated patch.
(In reply to Leyan from comment #22) > Created attachment 165642 [details] > updated patch checking object type for the different side panels > Hi In order for your patch to be incorporated into LibreOffice, it's also necessary for you to send a license statement to the developer mailing list at: https://lists.freedesktop.org/mailman/listinfo/libreoffice as documented on the developer wiki: https://wiki.documentfoundation.org/Development/GetInvolved#License_statement Could you please do this?
Even a possible patch is on its way, I want to to draw attention to the fact that bug 136898 seems to be another manifestation of this regression and the error logs in that post of further interest.
(In reply to NISZ LibreOffice Team from comment #23) > > Hi > > In order for your patch to be incorporated into LibreOffice, it's also > necessary for you to send a license statement to the developer mailing list > at: https://lists.freedesktop.org/mailman/listinfo/libreoffice > > as documented on the developer wiki: > https://wiki.documentfoundation.org/Development/GetInvolved#License_statement > > Could you please do this? Done here : https://lists.freedesktop.org/archives/libreoffice/2020-November/086237.html
I updated https://gerrit.libreoffice.org/c/core/+/102544 with the patch in https://bugs.documentfoundation.org/attachment.cgi?id=165642
*** Bug 136898 has been marked as a duplicate of this bug. ***
Ouyang Leyan committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/c1b96424b65aed2401a3e86d53a48cc12284de7c tdf#133630 Check object type when updating side panel data It will be available in 7.1.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.
Ouyang Leyan committed a patch related to this issue. It has been pushed to "libreoffice-7-0": https://git.libreoffice.org/core/commit/a5305624f21919352fa5d89c2cf259b0354d8a03 tdf#133630 Check object type when updating side panel data It will be available in 7.0.4. 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.
Xisco Fauli committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/6dae6c6fdcf410cb0258e04ed018c82d70e8cc3e tdf#133630: sc: Add UItest It will be available in 7.1.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.
essentially similar to what happened with this bug. Mainly graphical issues.
(In reply to Smyan Jaipuriyar from comment #31) > essentially similar to what happened with this bug. Mainly graphical issues. This bug is about a crash while bug 86321 is not.
I think we can close this issue now.
Ouyang Leyan, Andras Timar, thanks for fixing this issue
*** Bug 138331 has been marked as a duplicate of this bug. ***
*** Bug 138819 has been marked as a duplicate of this bug. ***