Bug 144843 - Selecting the whole table and then clicking background in the toolbar causes the LibreOffice app to crash.
Summary: Selecting the whole table and then clicking background in the toolbar causes ...
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
6.3.5.2 release
Hardware: Other All
: high major
Assignee: Armin Le Grand
URL:
Whiteboard: target:7.3.0 target:7.2.3
Keywords: bibisected, bisected, regression
Depends on:
Blocks:
 
Reported: 2021-09-30 18:01 UTC by Learner625
Modified: 2022-09-15 09:22 UTC (History)
6 users (show)

See Also:
Crash report or crash signature: ["SwDoc::GetBoxAttr(SwCursor const &,std::unique_ptr<SfxPoolItem,std::default_delete<SfxPoolItem> > &)"]


Attachments
Selecting the whole table and then clicking background in the toolbar causes the LibreOffice app to crash. (6.29 MB, video/x-matroska)
2021-09-30 18:01 UTC, Learner625
Details
testfile for transprent graphics & play with Glow/SoftEdge/ShadowBlur (53.62 KB, application/vnd.oasis.opendocument.graphics)
2022-09-15 09:22 UTC, Armin Le Grand
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Learner625 2021-09-30 18:01:16 UTC
Created attachment 175429 [details]
Selecting the whole table and then clicking background in the toolbar causes the LibreOffice app to crash.

Selecting the whole table and then clicking background in the toolbar causes the LibreOffice app to crash. And the the background is brought to the table toolbars by right swapping the background from the available commands to the assigned commands.
Comment 1 Roman Kuznetsov 2021-09-30 20:05:21 UTC
Interesting, I confirm the crash in

Version: 7.3.0.0.alpha0+ (x64) / LibreOffice Community
Build ID: fbfd91f2c5f4d66570c2d5a6f048b21f5d1671a4
CPU threads: 4; OS: Windows 6.1 Service Pack 1 Build 7601; UI render: Skia/Raster; VCL: win
Locale: ru-RU (ru_RU); UI: en-US
Calc: CL

and in 

Version: 6.3.5.2 (x86)
Build ID: dd0751754f11728f69b42ee2af66670068624673
CPU threads: 4; OS: Windows 6.1; UI render: GL; VCL: win; 
Locale: ru-RU (ru_RU); UI-Language: en-US
Calc: CL

but not in 6.2 => regression!

Steps:

Add to toolbar a Background tool (uno:BackgroundDialog)
Add a table into document
Select all the table
Click on the Background label (there is no icon for this UNO) on the toolbar
LO crashes


To Learner625 - I think you just want add a background to cells. Use for it a Table Cell Background Color tool on Table toolbar in bottom part of LibreOffice window when a text cursor is inside the table.
Comment 2 Learner625 2021-10-01 12:00:56 UTC
Comment on attachment 175429 [details]
Selecting the whole table and then clicking background in the toolbar causes the LibreOffice app to crash.

But using table properties and from there going to background doesn't create the crash.
Comment 3 raal 2021-10-09 16:33:23 UTC
This seems to have begun at the below commit.
Adding Cc: to Armin Le Grand ; Could you possibly take a look at this one?
Thanks
bibisect-linux-64-6.3$ 919fbf02a7ae264469801e9b8106690d0f380868 is the first bad commit
commit 919fbf02a7ae264469801e9b8106690d0f380868
Author: Jenkins Build User <tdf@pollux.tdf>
Date:   Fri Apr 26 04:48:01 2019 +0200

    source 1e2682235cded9a7cd90e55f0bfc60a1285e9a46

https://git.libreoffice.org/core/+/1e2682235cded9a7cd90e55f0bfc60a1285e9a46
    WIP: Further preparations for deeper Item changes
Comment 4 Xisco Faulí 2021-10-14 13:56:18 UTC
Hi Julien,
Would it be possible to have a backtrace here ?
Comment 5 Armin Le Grand 2021-10-14 15:51:00 UTC
Took a look. It crashes in sw/source/core/docnode/ndtbl1.cxx

        const sal_uInt16 nWhich = rToFill->Which();

because rToFill which is a std::unique_ptr<SfxPoolItem>& rToFill contains a nullptr. This is because it's called from SwFEShell::GetBoxBackground that already gets handed in a std::unique_ptr<SvxBrushItem>& rToFill which is also nullptr.
Origin is sw/source/uibase/shells/basesh.cxx where in SwBaseShell::ExecDlg in case FN_FORMAT_BACKGROUND_DLG in (currently) line 2716/2717 the following is done:

                std::unique_ptr<SvxBrushItem> aBrush;
                rSh.GetBoxBackground( aBrush );

Thus we have a code point where a nullptr is by purpose feeded to GetBoxBackground and gets - without check - dereferenced in SwDoc::GetBoxAttr. There are seven calls to GetBoxAttr in LO.

-> from which five DO create a default Item of some type that gets handed over.
-> from which two (one of these is used here) forward a unique_ptr.

It may be good to add a warning to SwDoc::GetBoxAttr when std::unique_ptr<SfxPoolItem>& rToFill is not filled (is nullptr). Those two cases are in file sw/source/core/frmedt/fetab.cxx. These are in methods

bool SwFEShell::GetBoxBackground( std::unique_ptr<SvxBrushItem>& rToFill ) const
bool SwFEShell::GetBoxDirection( std::unique_ptr<SvxFrameDirectionItem>& rToFill ) const

GetBoxBackground forwards he Item, so we need o check where that gets called. It has five calls from which one does not create an Item incarnation, at sw/source/uibase/shells/basesh.cxx:2717 (our crash here).

GetBoxDirection also forwards the Item, there are two calls, both create Items, so no problem.

I do not see a relationship to the changes mentioned as (1e2682235cded9a7cd90e55f0bfc60a1285e9a46) and which I did. That change did not remove any SfxPoolItem derived creations of items.

Thus the problem is a sw/source/uibase/shells/basesh.cxx:2717 - I will change and check.

Note: It is unfortunately no obvious that GetBoxBackground && GetBoxDirection *require* an incarnation of the intended SfxItem to be handed over - a bad trap for every user. People used o LO learned (the hard way) to check the called method...
Comment 6 Commit Notification 2021-10-15 15:16:23 UTC
Armin Le Grand (Allotropia) committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/e299f1f5e6097d9e5857dba65a65c33ac31974ae

tdf#144843 call to GetBoxBackground requires incarnated item

It will be available in 7.3.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.
Comment 7 Xisco Faulí 2021-10-18 09:04:09 UTC
Verified in

Version: 7.3.0.0.alpha0+ / LibreOffice Community
Build ID: 191e5fc227e40d18a1fe4563ed145517117596ea
CPU threads: 4; OS: Linux 5.10; UI render: default; VCL: gtk3
Locale: en-US (en_US.UTF-8); UI: en-US
Calc: threaded

@Armin, thanks for fixing this issue. Should it be closed as RESOLVED FIXED ?
Comment 8 Commit Notification 2021-10-18 10:36:15 UTC
Armin Le Grand (Allotropia) committed a patch related to this issue.
It has been pushed to "libreoffice-7-2":

https://git.libreoffice.org/core/commit/42b7c8c105602668f119d70f7c0aaf6e82aa9785

tdf#144843 call to GetBoxBackground requires incarnated item

It will be available in 7.2.3.

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.
Comment 9 Commit Notification 2021-10-18 13:16:19 UTC
Xisco Fauli committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/631ac203b926fd92401ff305bcc0d5dde6a24a2f

tdf#144843: sw: Add UItest

It will be available in 7.3.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.
Comment 10 Armin Le Grand 2022-09-15 09:22:46 UTC
Created attachment 182463 [details]
testfile for transprent graphics & play with Glow/SoftEdge/ShadowBlur