Bug 108919 - Assertion failed when inserting rectangle shape in spreadsheet
Summary: Assertion failed when inserting rectangle shape in spreadsheet
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
6.0.0.0.alpha0+
Hardware: All All
: medium normal
Assignee: Stephan Bergmann
URL:
Whiteboard: target:6.0.0
Keywords: haveBacktrace
Depends on:
Blocks: Crash-Assert
  Show dependency treegraph
 
Reported: 2017-07-03 03:41 UTC by Aron Budea
Modified: 2017-07-05 09:15 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments
bt with debug symbols (gtk3) (4.18 KB, text/plain)
2017-07-03 19:40 UTC, Julien Nabet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Aron Budea 2017-07-03 03:41:15 UTC
With a debug build, add a rectangle to the sheet.
As soon as you press left button on the sheet there is an assertion failed error.

This is the assert that fails:
assert(svl::detail::validRange(p.wid1, p.wid2));
http://opengrok.libreoffice.org/xref/core/svl/source/items/itemset.cxx#152

Both p.wid1 and p.wid2 are 0, that's the reason why validRange(...) returns false.
Those wid's are apparently WhichID's, and the following seems to be the line where they're initialized as 0 (and it fails a few levels deeper):

pObj->SetMergedItem( SvxAdjustItem( SvxAdjust::Center, 0 ) );
http://opengrok.libreoffice.org/xref/core/sc/source/ui/drawfunc/fuconcustomshape.cxx#202

I have no idea what this whole thing is about...

Observed using master build (ce6b877ba89ae4193f7a719a7dfa4bb45ecd04e4) / Windows 7.
Comment 1 Julien Nabet 2017-07-03 08:11:30 UTC
"Regression" from https://cgit.freedesktop.org/libreoffice/core/commit/?id=13bb5a4b09f5b2ad19dad1b55f45d0fe2b2fb908
I put quotes because perhaps it revealed a real bug.

Stephan: thought you might be interested in this one.
Comment 2 Aron Budea 2017-07-03 10:49:24 UTC
I assume that counts as confirmation, thanks Julien!
Comment 3 Julien Nabet 2017-07-03 19:40:18 UTC
Created attachment 134466 [details]
bt with debug symbols (gtk3)

On pc Debian x86-64 with master sources updated yesterday, I could reproduce this.
Comment 4 Stephan Bergmann 2017-07-04 08:30:31 UTC
That's interesting.  When doing <https://cgit.freedesktop.org/libreoffice/core/commit/?id=13bb5a4b09f5b2ad19dad1b55f45d0fe2b2fb908> "Make SfxItemSet ranges correct by construction" I naively assumed that an SfxItemSet's ranges would never include a WID=0, and that therefore the constructor with a single range,

> SfxItemSet( SfxItemPool&, sal_uInt16 nWhich1, sal_uInt16 nWhich2 );

should never be called with nWhich1=0 (just as for the multi-range constructor

> SfxItemSet( SfxItemPool&, int nWh1, int nWh2, int nNull, ... );

an argument of 0 signals end-of-ranges).

However, ever since <https://cgit.freedesktop.org/libreoffice/core/commit/?id=71b45e713f2426a70538e2620ee21fa18d82cc89> "INTEGRATION: CWS pchfix04", FuConstCustomShape::SetAttributes (sc/source/ui/drawfunc/fuconcustomshape.cxx) creates an SvxAdjustItem with WID=0, which is then passed into the SfxItemSet constructor as seen in the backtrace in attachment 134466 [details].

The other changes from that CWS pchfix04 (see "git log --patch --grep='#i73604#  eeitemid.hxx removed'" and "git log --patch --grep='#i73604# usage of ITEMID_\* removed'") create SvxAdjustItem instances with RES_PARATR_ADJUST or EE_PARA_JUST, it appears to be just this one exception with an explicit WID=0.

It is still unclear to me whether SfxItemSet instances containing WID=0 should e allowed, or whether the change in sc/source/ui/drawfunc/fuconcustomshape.cxx (done by Oliver) was wrong.  Oliver, any thoughts?
Comment 5 Oliver Specht (CIB) 2017-07-04 11:21:37 UTC
(In reply to Stephan Bergmann from comment #4)
> 
> It is still unclear to me whether SfxItemSet instances containing WID=0
> should e allowed, or whether the change in
> sc/source/ui/drawfunc/fuconcustomshape.cxx (done by Oliver) was wrong. 
> Oliver, any thoughts?
It should have been 
pObj->SetMergedItem( SvxAdjustItem( SVX_ADJUST_CENTER, EE_PARA_JUST ) );
Comment 6 Commit Notification 2017-07-05 09:04:11 UTC
Stephan Bergmann committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=ec1de6895d84fbe4f2d5fb7135a59a918138d970

tdf#108919: Fix WID, should be EE_PARA_JUST instead of 0

It will be available in 6.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 7 Stephan Bergmann 2017-07-05 09:15:44 UTC
(In reply to Oliver Specht (CIB) from comment #5)
> It should have been 
> pObj->SetMergedItem( SvxAdjustItem( SVX_ADJUST_CENTER, EE_PARA_JUST ) );

Thanks, fixed that now, see comment 6.

Oliver, any thoughts on "whether SfxItemSet instances containing WID=0 should be allowed" (i.e., whether <https://cgit.freedesktop.org/libreoffice/core/commit/?id=13bb5a4b09f5b2ad19dad1b55f45d0fe2b2fb908> "Make SfxItemSet ranges correct by construction" should be changed so that e.g.

  SfxItemSet(pool, {{0, 0}})

would generally be allowed)?