[Libreoffice-commits] core.git: Branch 'distro/collabora/cp-6.4' - sw/source

[Michael Stahl (via logerrit)]

 sw/source/core/layout/fly.cxx |   49 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 47 insertions(+), 2 deletions(-)

New commits:
commit f7f1dee0a5e016a61bf6d36a6a6b758d81ec6fa6
Author:     Michael Stahl <Michael.Stahl at cib.de>
AuthorDate: Thu Oct 22 19:17:24 2020 +0200
Commit:     Andras Timar <andras.timar at collabora.com>
CommitDate: Sat Oct 31 22:54:57 2020 +0100

    tdf#131679 sw: fix crash when copying fly via context menu
    
    sw::DocumentContentOperationsManager::CopyImplImpl() is called with a
    rPam that's on an SwOLENode.
    
    The problem (which i can't reproduce in --enable-dbgutil build,
    presumably for timing reasons) is that after the context menu pops up,
    some idle layout runs and reformats the document and deletes a
    SwFlyFrame and that calls SdrMarkView::UnmarkAllObj().
    
    Then when SwFEShell::Copy() is called, it finds IsFrameSelected()
    returns false, and it tries to copy normal text when the cursor is on
    an SwOLENode.
    
    Fix this in SwFlyFrame::FinitDrawObj() by first moving the cursor out
    of any selected flys.
    
    (regression from 81ec0039b2085faab49380c7a56af0c562d4c9e4
     - previously CopyImplImpl() would return early)
    
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/104697
    Tested-by: Jenkins
    Reviewed-by: Michael Stahl <michael.stahl at cib.de>
    (cherry picked from commit 40bff2567fa4a3fa8ec4445182cbbe3547c17410)
    
    tdf#131679 sw: follow-up: Unmark before SetSelection()
    
    Backporting this to 6.4, it crashes in CppunitTest_desktop_lib because
    some sidebar is loaded from SwView::AttrChangedNotify()/SelectShell()
    and that ends up calling SwView::StateTabWin() about 40 stack frames
    later and this calls SwFEShell::GetAnyCurRect() which gets the still
    selected fly but its page frame is null.
    
    So make sure shells don't see the deleted fly.
    
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/104815
    Tested-by: Jenkins
    Reviewed-by: Michael Stahl <michael.stahl at cib.de>
    (cherry picked from commit f63afb95b5c2d80d33a35820ef1d9abd9e70d3ca)
    
    Change-Id: Id135fcc002c03c07c34fbdc0355f2895d8b6565b
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/104682
    Tested-by: Jenkins
    Reviewed-by: Caolán McNamara <caolanm at redhat.com>
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/105095
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
    Reviewed-by: Andras Timar <andras.timar at collabora.com>

diff --git a/sw/source/core/layout/fly.cxx b/sw/source/core/layout/fly.cxx
index b5f364511f6e..190e799dbc56 100644
--- a/sw/source/core/layout/fly.cxx
+++ b/sw/source/core/layout/fly.cxx
@@ -377,6 +377,32 @@ void SwFlyFrame::InitDrawObj()
                                 : nHellId );
 }
 
+static SwPosition ResolveFlyAnchor(SwFrameFormat const& rFlyFrame)
+{
+    SwFormatAnchor const& rAnch(rFlyFrame.GetAnchor());
+    if (rAnch.GetAnchorId() == RndStdIds::FLY_AT_PAGE)
+    {   // arbitrarily pick last node
+        return SwPosition(SwNodeIndex(rFlyFrame.GetDoc()->GetNodes().GetEndOfContent(), -1));
+    }
+    else
+    {
+        SwPosition const*const pPos(rAnch.GetContentAnchor());
+        assert(pPos);
+        if (SwFrameFormat const*const pParent = pPos->nNode.GetNode().GetFlyFormat())
+        {
+            return ResolveFlyAnchor(*pParent);
+        }
+        else if (pPos->nContent.GetIdxReg())
+        {
+            return *pPos;
+        }
+        else
+        {
+            return SwPosition(*pPos->nNode.GetNode().GetContentNode(), 0);
+        }
+    }
+}
+
 void SwFlyFrame::FinitDrawObj()
 {
     if(!GetVirtDrawObj() )
@@ -391,8 +417,27 @@ void SwFlyFrame::FinitDrawObj()
             for(SwViewShell& rCurrentShell : p1St->GetRingContainer())
             {   // At the moment the Drawing can do just do an Unmark on everything,
                 // as the Object was already removed
-                if(rCurrentShell.HasDrawView() )
-                    rCurrentShell.Imp()->GetDrawView()->UnmarkAll();
+                if (rCurrentShell.HasDrawView() &&
+                    rCurrentShell.Imp()->GetDrawView()->GetMarkedObjectList().GetMarkCount())
+                {
+                    if (SwFEShell *const pFEShell = dynamic_cast<SwFEShell*>(&rCurrentShell))
+                    {   // tdf#131679 move any cursor out of fly
+                        SwFlyFrame const*const pOldSelFly = ::GetFlyFromMarked(nullptr,  pFEShell);
+                        rCurrentShell.Imp()->GetDrawView()->UnmarkAll();
+                        if (pOldSelFly)
+                        {
+                            SwPosition const pos(ResolveFlyAnchor(*pOldSelFly->GetFormat()));
+                            SwPaM const temp(pos);
+                            pFEShell->SetSelection(temp);
+                            // could also call SetCursor() like SwFEShell::SelectObj()
+                            // does, but that would access layout a bit much...
+                        }
+                    }
+                    else
+                    {
+                        rCurrentShell.Imp()->GetDrawView()->UnmarkAll();
+                    }
+                }
             }
         }
     }