| Summary: | Editing a conditional format from the Manage dialog changes the range / creates a new one | ||
|---|---|---|---|
| Product: | LibreOffice | Reporter: | Stéphane Guillou (stragu) <stephane.guillou> |
| Component: | Calc | Assignee: | Armin Le Grand <Armin.Le.Grand> |
| Status: | VERIFIED FIXED | ||
| Severity: | normal | CC: | Armin.Le.Grand, miguelangelrv |
| Priority: | medium | Keywords: | bibisected, bisected, regression |
| Version: | 24.8.0.0 alpha0+ | ||
| Hardware: | x86-64 (AMD64) | ||
| OS: | All | ||
| See Also: | https://bugs.documentfoundation.org/show_bug.cgi?id=160250 | ||
| Whiteboard: | target:24.8.0 | ||
| Crash report or crash signature: | Regression By: | Armin Le Grand | |
| Bug Depends on: | |||
| Bug Blocks: | 116221 | ||
| Attachments: | sample ODS | ||
|
Description
Stéphane Guillou (stragu)
2024-03-18 05:21:47 UTC
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. |