Description: (I already have a fix, but I'm opening this to provide a detailed description) Steps to Reproduce: 1. Open the attached file 'chart.ods' 2. Open a new Calc window 3. Open the Navigator and change the drag mode to 'Insert as Copy' 4. At the bottom of the Navigator, select 'chart (inactive)' 5. Unfold 'OLE objects' and drag&drop 'Object 1' to the sheet Actual Results: The chart is not inserted. Only it's contour is visible. Expected Results: The chart should be visible Reproducible: Always User Profile Reset: Yes OpenGL enabled: Yes Additional Info: This is a regression. A bibisect shows that the first bad commit is 91b0d2122bde (https://gerrit.libreoffice.org/#/c/53933/) After a good time debugging, I found that in svx/source/svdraw/svdxcgv.cxx:745 the argument given to CloneSdrObject is not the correct one.
Created attachment 151700 [details] chart.ods
The fix: https://gerrit.libreoffice.org/#/c/73026/
confirming
Not reproducible in Version: 5.2.0.0.alpha1+ Build ID: 5b168b3fa568e48e795234dc5fa454bf24c9805e CPU Threads: 4; OS Version: Linux 4.15; UI Render: default; Locale: ca-ES (ca_ES.UTF-8)
This is reproducible in Version: 6.3.0.0.beta1 (x64) Build ID: a187af327633f5f00363be5131bd21a13e0f1a7b CPU threads: 2; OS: Windows 10.0; UI render: default; VCL: win; Locale: en-US (en_US); UI-Language: en-US Calc: threaded and in Version: 6.1.0.1 (x64) Build ID: 378e26bd4f22a135cef5fa17afd5d4171d8da21a CPU threads: 2; OS: Windows 10.0; UI render: default; Locale: en-US (en_US); Calc: group threaded but not in Version: 6.0.7.3 (x64) Build ID: dc89aa7a9eabfd848af146d5086077aeed2ae4a5 CPU threads: 2; OS: Windows 10.0; UI render: default; Locale: en-US (en_US); Calc: group I did a bibisect in branch 6.1 and found that the first bad commit is 91b0d2122bde (https://gerrit.libreoffice.org/#/c/53933/)
Created attachment 151912 [details] 6.1_bibisect.log Here is the bibisect log
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 author Armin Le Grand <Armin.Le.Grand@cib.de> 2018-05-07 11:44:26 +0200 committer Armin Le Grand <Armin.Le.Grand@cib.de> 2018-05-08 01:53:46 +0200 commit 91b0d2122bdee361bf5412a42d48ff051159cbf2 (patch) tree 003ef60b93668847803a812486534ebc805e6546 parent bdccb7e9991d83029eb2f2f11327b54534a00db8 (diff) tdf#116977 secured ::Clone methods
Andrés Maldonado committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/+/e4cea049c80f4fd6d2a586e73fe9fa08ebd08371%5E%21 tdf#125520 Fix OLE objects drag&drop with 'Insert as Copy' It will be available in 6.4.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.
A polite ping to Andrés Maldonado: Is this bug fixed? if so, could you please close it as RESOLVED FIXED ? Otherwise, Could you please explain what's missing? Thanks
Xisco Faulí committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/09e6824bc868990095233825c415556399dd0652 tdf#130614: Revert "tdf#125520 Fix OLE objects drag&drop with 'Insert as Copy'" It will be available in 7.0.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.
Xisco Faulí committed a patch related to this issue. It has been pushed to "libreoffice-6-4": https://git.libreoffice.org/core/commit/d3e77bd112c3f7084b0dd92c36ff60f1fd1ddc2f tdf#130614: Revert "tdf#125520 Fix OLE objects drag&drop with 'Insert as Copy'" It will be available in 6.4.2. 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.
Xisco Faulí committed a patch related to this issue. It has been pushed to "libreoffice-6-4-1": https://git.libreoffice.org/core/commit/25937732b9ef881651611f99d4178e90888642a2 tdf#130614: Revert "tdf#125520 Fix OLE objects drag&drop with 'Insert as Copy'" It will be available in 6.4.1. 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.
grepping, looks as if just target SdrModel is wrong...
svx/source/svdraw/svdxcgv.cxx:745 is the wrong place - it creates the clone that is visualized during D&D itself. The real clone gets created in SdrExchangeView::Paste line 300: SdrObject* pNewObj(pSrcOb->CloneSdrObject(*mpModel)); All the target SdrModel ptrs are correct. Scenario is as follows: (1) D&D starts, a temp SdrModel gets created with a single page, the SdrModel gets cloned to there (svdxcgv.cxx:745) (2) At end of D&D that clone gets cloned to target SdrModel and SdrPage Using such a simplified SdrModel for D&D/Copy&Paste is standard - it gets saved/loaded in clipboard using that mechanism. Problem seems more that already at the 1st clone the EmbeddedObjectContainer stuff gets not copied because ::comphelper::IEmbeddedHelper* pDestPers(getSdrModelFromSdrObject().GetPersist()); ::comphelper::IEmbeddedHelper* pSrcPers(rObj.getSdrModelFromSdrObject().GetPersist()); if( pDestPers && pSrcPers ) { nullptr == pDestPers which is probably because the temp SdrModel does indeed not have Persist data (aka associated file AFAIK?). Thus my guess to fix that is that the in-between SDrModel created at SdrExchangeView::CreateMarkedObjModel()::703 somehow would need a Persist in OLE cases...?
Comment in code: // tdf#125520 - temp SdrModel will need a comphelper::IEmbeddedHelper // to succesfully clone the OLE content, use the one from source model // in the temporary SdrModel - it gets not deleted in SdrModel destructor. // As long as the temporary SdrModel is used temprary (and does NOT get // extended to a full document) this *should* work. There stay some // concerns about what may happen in BG and if saved/loaded from clipboard, // so this *might* need to be enhanced in the future. Works now with 2nd chart (tried Draw/Impress/Writer, but not possible there using Navigator), so let's use this
Armin Le Grand committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/d08d5c1857482cb3789ed2896921abeb83f4d217 tdf#125520 enhance internal OLE cloning It will be available in 7.0.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.
Verified in Version: 7.0.0.0.alpha0+ Build ID: a11c10a83f6fceae6cfb519725d06f8eaf1013fb CPU threads: 4; OS: Linux 4.19; UI render: default; VCL: gtk3; Locale: en-US (en_US.UTF-8); UI-Language: en-US Calc: threaded @Armin, thanks for fixing this issue!!
Hi Armin, Actually your commit < d08d5c1857482cb3789ed2896921abeb83f4d217 > reintroduced bug 130614. Changing it to REOPENED. Could you please take a look ?
Hi Xisco - will have a look ASAP
This is really bad - like a hen/egg problem :-( The temp model has no own comphelper::IEmbeddedHelper, so it cannot clone chart contents. If using the one from source document, it will die when that gets closed - sigh...
There are already some quite 'creative' parts in that problem area, when looking at ::AllocModel, e.g. SdDrawDocument::AllocModel() uses AllocSdDrawDocument() which uses mpCreatingTransferable and comments like // Document is created for drag & drop/clipboard. To be able to // do this, the document has to know a DocShell (SvPersist). show that this is not the 1st problem of this kind. Calc uses something like static SfxObjectShell* pGlobalDrawPersist; // for AllocModel which gets set once and then consumed, see ScDrawLayer::SetGlobalDrawPersist and usages - a global, use-once parameter - argh. But this leads at least to ScDrawLayer::AllocModel() and pGlobalDrawPersist would be used if it would have been set. Checking where this is set (ScDrawLayer::SetGlobalDrawPersist)...
It is e.g. used in ScDrawView::BeginDrag() in sc\source\ui\view\drawvie4.cxx where also the case using CTRL+C starts which is ScDrawView::DoCopy(). Thus the former change in SdrExchangeView::CreateMarkedObjModel() pNewModel->SetPersist(mpModel->GetPersist()); is even wriong - it replaces the already set persist with the source document. This all shows that this fix was wrong. It seems as if there are missing mechanisms for the original case in the task - to start D&D using the Navigator. Checking...
Adapted lcl_DoDragObject in sc\source\ui\navipi\content.cxx as needed to support the existing SetGlobalDrawPersist mechanism. Removed changes in SdrExchangeView::CreateMarkedObjModel() which were wrong and replaced with a warning if no Persist is given and cloned object is an OLE. Change is on gerrit (see https://gerrit.libreoffice.org/c/core/+/90263).
Armin Le Grand committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/f9e2cfc0d2e244c1550a0e2cc8de960f82eaf9cf tdf#125520 create a persist correctly for OLE It will be available in 7.0.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.
Issue verified in Version: 7.0.0.0.alpha0+ Build ID: dde0cf9eafed91c88d9999a294ea32fe0e8aa4bc CPU threads: 4; OS: Linux 4.19; UI render: default; VCL: gtk3; Locale: en-US (en_US.UTF-8); UI-Language: en-US Calc: threaded @Armin, thanks for fixing this issue! Can we close it as RESOLVED FIXED ?
Armin Le Grand committed a patch related to this issue. It has been pushed to "libreoffice-6-4": https://git.libreoffice.org/core/commit/90f57175abf31ebad850c9653b41eccd230893b8 tdf#125520 create a persist correctly for OLE It will be available in 6.4.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.