Bug 120728 - crash in report editing: insert page number in footer while header is active
Summary: crash in report editing: insert page number in footer while header is active
Status: CLOSED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
6.1.2.1 release
Hardware: x86-64 (AMD64) All
: highest critical
Assignee: Armin Le Grand (CIB)
URL:
Whiteboard: target:6.2.0 target:6.1.4
Keywords: bibisected, bisected, regression
: 121351 (view as bug list)
Depends on:
Blocks: AW080-Regressions
  Show dependency treegraph
 
Reported: 2018-10-20 12:27 UTC by Regina Henschel
Modified: 2018-11-26 17:03 UTC (History)
5 users (show)

See Also:
Crash report or crash signature: ["rptui::OReportController::createControl(com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const &,com::sun::star::uno::Reference<com::sun::star::report::XSection> const &,rtl::OUString const &,unsigned short)"]


Attachments
"before" database to test (10.37 KB, application/vnd.sun.xml.base)
2018-10-20 12:27 UTC, Regina Henschel
Details
call stack as reported by Visual Studio (5.36 KB, text/plain)
2018-10-20 12:28 UTC, Regina Henschel
Details
Crash as seen in VS2017 (154.56 KB, image/png)
2018-10-20 12:29 UTC, Regina Henschel
Details
"after" database after the crash (10.37 KB, application/vnd.sun.xml.base)
2018-10-20 12:29 UTC, Regina Henschel
Details
bt with debug symbols (10.21 KB, text/plain)
2018-10-20 16:31 UTC, Julien Nabet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Regina Henschel 2018-10-20 12:27:53 UTC
Created attachment 145853 [details]
"before" database to test

How to reproduce:
Open attached document "before". It is a database file with embedded HSQLDB. It has one table and one report. Always when you ask to convert the DB, click on "Later".
Open the report for editing.
Click in header area to make it active. It then has a white border on left side.
Use menu Insert > Page Number...
Make the settings "Page N of M", "Bottom of Page", "Alignment right".
Click OK --> Crash.

After finished all crash error messages and notices start LibreOffice again and open the document again. I got no recovery. After the file is opened click on section "Table". Error: Connection could not be established. That means the database is corrupt and all data is lost.
Comment 1 Regina Henschel 2018-10-20 12:28:27 UTC
Created attachment 145854 [details]
call stack as reported by Visual Studio
Comment 2 Regina Henschel 2018-10-20 12:29:05 UTC
Created attachment 145855 [details]
Crash as seen in VS2017
Comment 3 Regina Henschel 2018-10-20 12:29:40 UTC
Created attachment 145856 [details]
"after" database after the crash
Comment 4 Regina Henschel 2018-10-20 13:02:16 UTC
It crashes too, if the footer section is active while insert page number in footer.
Comment 5 Drew Jensen 2018-10-20 13:12:10 UTC
Confirmed with Ubuntu 18.04 and LibreOffice 6.2Alpha0
Crash report included.
Comment 6 Regina Henschel 2018-10-20 14:03:03 UTC
It was OK in Version: 5.4.7.2 (x64)
Build ID: c838ef25c16710f8838b1faec480ebba495259d0
CPU threads: 8; OS: Windows 6.19; UI render: default; 
Locale: en-US (en_US); Calc: CL
Comment 7 Drew Jensen 2018-10-20 14:07:12 UTC
(In reply to Regina Henschel from comment #6)
> It was OK in Version: 5.4.7.2 (x64)
> Build ID: c838ef25c16710f8838b1faec480ebba495259d0
> CPU threads: 8; OS: Windows 6.19; UI render: default; 
> Locale: en-US (en_US); Calc: CL

Also using Ubuntu 18.04 - it works as expected in 6.0.7 and crashes in 6.1.2
Comment 8 Julien Nabet 2018-10-20 16:31:26 UTC
Created attachment 145860 [details]
bt with debug symbols

On pc Debian x86-64 with master sources updated today, I could reproduce this.

It may be due to https://cgit.freedesktop.org/libreoffice/core/commit/?id=f2bd68c33c13b94cf844227d04a7eba9fe3723f8
Comment 9 Julien Nabet 2018-10-20 16:47:13 UTC
I submitted this patch to review:
https://gerrit.libreoffice.org/#/c/62082/1

See comments of this one.
In brief, perhaps my previous patch on tdf#120152 was wrong but I had no feedback about it.
Comment 10 Regina Henschel 2018-10-20 20:11:59 UTC
Hi Julien, with that patch I can insert a page number field. But I think, there is still something wrong. If a try to insert a shape, I get a crash with the unpatched version and an assertion with your patch. Tested on master.
Assertion failed!
Program: ..\core\instdir\program\rptuilo.dll
File: .. /Reference.h
Line:420

Expression: _pInterface != NULL
Comment 11 Julien Nabet 2018-10-20 21:58:54 UTC
(In reply to Regina Henschel from comment #10)
> Hi Julien, with that patch I can insert a page number field. But I think,
> there is still something wrong. If a try to insert a shape, I get a crash
> with the unpatched version and an assertion with your patch. Tested on
> master.
> Assertion failed!
> Program: ..\core\instdir\program\rptuilo.dll
> File: .. /Reference.h
> Line:420
> 
> Expression: _pInterface != NULL

Indeed, I could reproduce the assert with master sources + patch.

I could also reproduce this with LO Debian package 6.1.3.1 so before all of my patches were there since none has been included in 6.1 branch for the moment.
Comment 13 Xisco Faulí 2018-10-21 17:05:57 UTC
Regression introduced by:

https://cgit.freedesktop.org/libreoffice/core/commit/?id=4b4942224b550235da228655677b5c068a053254

author	Armin Le Grand <Armin.Le.Grand@cib.de>	2018-04-16 22:34:50 +0200
committer	Armin Le Grand <Armin.Le.Grand@cib.de>	2018-05-25 12:31:32 +0200
commit	4b4942224b550235da228655677b5c068a053254 (patch)
tree	a660a04a1f7a3eee910da780ece271d68942201d
parent	f8edef392245c292398a80f6a858ca19f32df9c3 (diff)
SOSAW080: Derive SdrObjGroup from SdrObjList

Bisected with: bibisect-linux64-6.2

Adding Cc: to Armin Le Grand
Comment 14 Commit Notification 2018-10-24 12:31:19 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

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

tdf#120728: retrieve label page

It will be available in 6.2.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 15 Commit Notification 2018-10-30 09:23:18 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/f88b95032262c434647475b6af7e33068635b6c4%5E%21

Revert tdf#120782, tdf#120728, tdf#120152, tdf#120151

It will be available in 6.2.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 16 Julien Nabet 2018-10-30 09:30:21 UTC
Should fail again since I reverted my patch.
Indeed, it brought some regression.
Comment 17 Armin Le Grand (CIB) 2018-10-31 13:05:21 UTC
Can reproduce
Comment 18 Armin Le Grand (CIB) 2018-10-31 13:30:39 UTC
Similar to other stuff:

svxcorelo.dll!SvxDrawPage::CreateShapeByTypeAndInventor(unsigned short nType, SdrInventor nInventor, SdrObject * pObj, SvxDrawPage * mpPage, const rtl::OUString & referer) Line 795 (c:\lo\work04_64\svx\source\unodraw\unopage.cxx:795)
svxcorelo.dll!SdrObject::getUnoShape() Line 2812 (c:\lo\work04_64\svx\source\svdraw\svdobj.cxx:2812)
rptlo.dll!rptui::OObjectBase::getUnoShapeOf(SdrObject & _rSdrObject) Line 462 (c:\lo\work04_64\reportdesign\source\core\sdr\RptObject.cxx:462)
rptlo.dll!rptui::OUnoObject::getUnoShape() Line 896 (c:\lo\work04_64\reportdesign\source\core\sdr\RptObject.cxx:896)
rptuilo.dll!rptui::OReportController::createControl(const com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> & _aArgs, const com::sun::star::uno::Reference<com::sun::star::report::XSection> & _xSection, const rtl::OUString & _sFunction, unsigned short _nObjectId) Line 3167 (c:\lo\work04_64\reportdesign\source\ui\report\ReportController.cxx:3167)

At the SdrObject (a rptui::OUnoObject) no SdrPage is set die to not being added, so

SdrObject::getUnoShape()

uses the static call

SvxDrawPage::CreateShapeByTypeAndInventor

instead of the dynamic one above it. That call does not know the 'ReportDesign' inventor and fails.

One idea is that a factory that does have no possibility to add creators (here: for ReportDesign) is a bad design by itself.

Another one is the usual - debug the old code, check where/when the SDrObject gets added to a SdrPage and see if this can be done early...
Comment 19 Armin Le Grand (CIB) 2018-10-31 14:24:22 UTC
In lo5.2 was implicitely inserted in

void correctOverlapping(SdrObject* _pControl,OReportSection& _aReportSection,bool _bInsert)

where _bInsert is defaulted to true. It is not explicitely inserted when called at the end of

void OReportController::createControl(const Sequence< PropertyValue >& _aArgs,const uno::Reference< report::XSection>& _xSection,const OUString& _sFunction,sal_uInt16 _nObjectId)

Thus looks as if inserting to a SdrPage was seen as not so relevant Model-defining action (?) which is bad.
The fix tdf#120674 does not help here due to being specialized for end of interactive creation.
For those SdrObject created in ReportDesign editor there seems to be no well-designed place where to insert the created model objects (probably due to that effect that a SdrPage-ptr was already set in old days without the SdrObject being inserted what was inconsistent).

OTOH the factory (DlgEdFactory, see SdrObject::getUnoShape()) will only do the right thing when getSdrPageFromSdrObject() does something useful. This is not a good design anyways, maybe best solution is to correct that one to get a SdrPage as input parameter. SdrObject::getUnoShape() should probably work without the SdrObject being inserted to a SdrPage already.
That again raises the question: Is SvxDrawPage::CreateShape the right place at all? Should a factory method fo shape creation not better be placed at the SdrModel/XModel? Or the other way around: When being added to the Page, will it ever make  sense to create different objects for different pages in the same model?

But that goes too far - will not move ::CreateShape implementations to model derivations, even when that would make sense.

For now, will take a closer look at maybe giving SdrObject::getUnoShape() a parameter for the SdrPage with the purpose to get an incarnation of such a SdrPage for the only purpose to be able to call the overloaded ::CreateShape or ::CreateShapeByTypeAndInventor at the correct SdrPage derivation...
Comment 20 Armin Le Grand (CIB) 2018-10-31 19:55:37 UTC
Hard stuff, analyzed. Still there is a relatively 'simple' solution. Added that and quite some comments. 1st veraion of fix at https://gerrit.libreoffice.org/#/c/62710/
Looks as if that also fixes tdf#120152.
tdf#120674 is not influenced and still needed - that one was about not getting the SdrObjects inserted/initialized.
Comment 21 Commit Notification 2018-11-01 09:59:48 UTC
Armin Le Grand committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/52bbb04f1e39b2d778275c91f77b6c0714ecd0d0%5E%21

tdf#120728 support SvxShape for SdrShape if not inserted

It will be available in 6.2.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 22 Armin Le Grand (CIB) 2018-11-01 11:38:28 UTC
Re-checked, works well.
Comment 23 Xisco Faulí 2018-11-01 13:29:45 UTC
Verified in

Version: 6.2.0.0.alpha1+
Build ID: 4326fb3ef3ddd7c6f9d08ba96add4f4736503ceb
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); Calc: threaded

@Armin, Thanks for fixing this!!

Cherry-picked to 6.1 -> https://gerrit.libreoffice.org/#/c/62738/
Comment 24 Commit Notification 2018-11-01 18:36:17 UTC
Armin Le Grand committed a patch related to this issue.
It has been pushed to "libreoffice-6-1":

https://git.libreoffice.org/core/+/f05a551a6b49916241696481567e32b320c6749b%5E%21

tdf#120728 support SvxShape for SdrShape if not inserted

It will be available in 6.1.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.
Comment 25 Regina Henschel 2018-11-06 00:22:06 UTC
Verified in Version: 6.1.4.0.0+ (x64)
Build ID: 502077fb9a9387a5aac85dabf445c10718309611
CPU threads: 8; OS: Windows 10.0; UI render: GL; 
TinderBox: Win-x86_64@42, Branch:libreoffice-6-1, Time: 2018-11-05_13:11:30
Locale: en-US (en_US); Calc: CL
Comment 26 Julien Nabet 2018-11-11 13:57:23 UTC
*** Bug 121351 has been marked as a duplicate of this bug. ***