Created attachment 193169 [details] sample ODS Steps: 1. Open attached ODS 2. Format > Conditional > Manage > Edit (opens the dialog to edit the first conditional format) > Cancel 3. Click Edit again Result A: Range changes to active cell or active selection; creates a new rule if pressing OK. 4. Cancel Result 2: Manage dialog closes. Expected results: range does not change, rule is edited instead of creating a new one, Manage dialog stays open. Bibisected with linux-64-24.8 repo to first bad build [7f212cbb132606387f165be768f859dd36486c08] which points to: commit 74a56b7434047b6ffbf865af60dba98a9273b63a author Armin Le Grand (allotropia) Fri Jan 19 15:42:34 2024 +0100 committer Armin Le Grand Sun Jan 21 12:56:22 2024 +0100 ITEM: solve ScCondFormatDlgItem situation Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162313 Repro in: Version: 24.8.0.0.alpha0+ (X86_64) / LibreOffice Community Build ID: 39663a323c3330c18b610fcdc9e9c75ddac770f1 CPU threads: 8; OS: Linux 6.5; UI render: default; VCL: gtk3 Locale: en-AU (en_AU.UTF-8); UI: en-US Calc: CL threaded Same with gen VCL plugin. Armin, can you please have a look?
Repro Version: 24.8.0.0.alpha0+ (X86_64) / LibreOffice Community Build ID: 3ba85b7786663da4f2de1a3c2fe7ee9a27657293 CPU threads: 16; OS: Windows 10.0 Build 22631; UI render: Skia/Raster; VCL: win Locale: es-ES (es_ES); UI: en-US Calc: CL threaded
Taking a look. GHas probably to do with ScCondFormatDlgData lifetime, was hard to figure out when doing mentioned change. May also be ScConditionalFormatList which is a member...
Tried to create a compare-version, but revert is too unmaintainable, going to version direct before that commit...
This is really hard to debug with the async dialog that gets triggered by a callback mechanism that also uses the data in question. Compared with formally used Item's lifetime and it seems I did too much cleanup. There are calls to pTabViewShell->setScCondFormatDlgItem(nullptr); that replace the formally used GetPool().DirectRemoveItemFromPool(*pDlgItem); but that kills the shared_ptr'ed ScCondFormatDlgData and thus it's no longer available. This is not needed anymore when not using that formally DirectAdd/RemoveItem stuff, so should be just good to rely on the shared_ptr of the pTabViewShell...
Thinking about it that way: If that would have stayed an Item I would have used a SfxPoolItemHolder at the pTabViewShell. This frees of taking care/managing the lifetime of Items yourself. Now using a shared_ptr does the same thing -> do not try to clean it up, that part is automaitically done...
Should be done with https://gerrit.libreoffice.org/c/core/+/165552. @stephane.guillou@libreoffice.org: Thanks for digging that out! I do not know that area of SC too well, so please - if possible - check that dialog and what can be done with it. Thanks in advance!
(In reply to Armin Le Grand from comment #6) > Should be done with https://gerrit.libreoffice.org/c/core/+/165552. > > @stephane.guillou@libreoffice.org: Thanks for digging that out! I do not > know that area of SC too well, so please - if possible - check that dialog > and what can be done with it. Thanks in advance! Thank you for looking into it, Armin! I tested the patch in my own build with gen and gtk3 VCL plugins, it fixes the bug. I see on gerrit that the test UITest_conditional_format > test_tdf105351_cond_format_data_bar (for bug 105351) failed though.
@stephane.guillou@libreoffice.org: Thanks for checking - did you just do the check in description or some more...? Yupp, seems - in some cases - more cleanup is wanted. Just would have to know when :-/ Will have to experiment...
Armin Le Grand (allotropia) committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/387a9c445793e8377f85e508d935dc070fd8ab74 tdf#160252 ITEM remove unnecessary cleanups of shared_ptr 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.
Adapted to needed cleanups (did one too much as it seems). Should be good with https://gerrit.libreoffice.org/c/core/+/165552
Thanks Armin. Verified in: Version: 24.8.0.0.alpha0+ (X86_64) / LibreOffice Community Build ID: 30c6e51fc9cb0fa864e1755271343d847fcced25 CPU threads: 8; OS: Linux 6.5; UI render: default; VCL: gtk3 Locale: en-AU (en_AU.UTF-8); UI: en-US Calc: CL threaded (I assume the removal of the whiteboard target was an error?) (In reply to Armin Le Grand from comment #8) > @stephane.guillou@libreoffice.org: Thanks for checking - did you just do the > check in description or some more...? I had a poke around with removing rules, editing, opening and cancelling, adding new ones... Seems to all work fine now.