sw/qa/core/layout/data/floattable-next-leaf-in-section.docx |binary
 sw/qa/core/layout/flycnt.cxx                                |   11 +++
 sw/qa/extras/tiledrendering/tiledrendering.cxx              |   42 ++++++++++++
 sw/source/core/layout/flycnt.cxx                            |   14 ++++
 sw/source/core/undo/docundo.cxx                             |    2 
 5 files changed, 68 insertions(+), 1 deletion(-)

New commits:
commit 6644d451bd69c2bc87c8441b5e2d9374bd0f1763
Author:     Miklos Vajna <vmik...@collabora.com>
AuthorDate: Wed May 24 15:40:35 2023 +0200
Commit:     Andras Timar <andras.ti...@collabora.com>
CommitDate: Thu May 25 09:07:51 2023 +0200

    sw: fix wrong downcast in UndoManager::IsViewUndoActionIndependent()
    
    In case a user types in one view, an other user types in an other view,
    finally the first user deletes, then getting the undo state resulted in
    a memory corruption.
    
    This went wrong in commit 2875c65946e59f5dd7968155463bf00bd71d440b (sw,
    out of order undo: allow a subset of a non-empty redo list, 2021-11-11),
    the intention was to check if we have a redo item and it has the
    expected type, but we checked the type of an earlier undo action that
    belongs to the view.
    
    Fix the problem by checking the type of the correct undo action, this
    was probably a copy&paste error of mine.
    
    Resolves <https://github.com/CollaboraOnline/online/issues/6412>.
    
    Change-Id: I6cb2a081067695a045d86b4ef427cc5a76c0f9c4
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/152205
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
    Reviewed-by: Andras Timar <andras.ti...@collabora.com>

diff --git a/sw/qa/extras/tiledrendering/tiledrendering.cxx 
b/sw/qa/extras/tiledrendering/tiledrendering.cxx
index 15854f194f2a..18096767a570 100644
--- a/sw/qa/extras/tiledrendering/tiledrendering.cxx
+++ b/sw/qa/extras/tiledrendering/tiledrendering.cxx
@@ -1321,6 +1321,48 @@ CPPUNIT_TEST_FIXTURE(SwTiledRenderingTest, 
testUndoReorderingRedo)
     SfxViewShell::Current()->setLibreOfficeKitViewCallback(nullptr);
 }
 
+CPPUNIT_TEST_FIXTURE(SwTiledRenderingTest, testUndoReorderingRedo2)
+{
+    // Create two views.
+    SwXTextDocument* pXTextDocument = createDoc();
+    SwWrtShell* pWrtShell1 = pXTextDocument->GetDocShell()->GetWrtShell();
+    int nView1 = SfxLokHelper::getView();
+    int nView2 = SfxLokHelper::createView();
+    
pXTextDocument->initializeForTiledRendering(uno::Sequence<beans::PropertyValue>());
+    SwWrtShell* pWrtShell2 = pXTextDocument->GetDocShell()->GetWrtShell();
+
+    // Type in the first view.
+    SfxLokHelper::setView(nView1);
+    pWrtShell1->SttEndDoc(/*bStt=*/true);
+    pXTextDocument->postKeyEvent(LOK_KEYEVENT_KEYINPUT, 'f', 0);
+    pXTextDocument->postKeyEvent(LOK_KEYEVENT_KEYUP, 'f', 0);
+    Scheduler::ProcessEventsToIdle();
+
+    // Type to the same paragraph in the second view.
+    SfxLokHelper::setView(nView2);
+    pWrtShell2->SttEndDoc(/*bStt=*/true);
+    pXTextDocument->postKeyEvent(LOK_KEYEVENT_KEYINPUT, 's', 0);
+    pXTextDocument->postKeyEvent(LOK_KEYEVENT_KEYUP, 's', 0);
+    Scheduler::ProcessEventsToIdle();
+
+    // Delete in the first view and undo.
+    SfxLokHelper::setView(nView1);
+    pXTextDocument->postKeyEvent(LOK_KEYEVENT_KEYINPUT, 0, 
awt::Key::BACKSPACE);
+    pXTextDocument->postKeyEvent(LOK_KEYEVENT_KEYUP, 0, awt::Key::BACKSPACE);
+    Scheduler::ProcessEventsToIdle();
+    dispatchCommand(mxComponent, ".uno:Undo", {});
+    Scheduler::ProcessEventsToIdle();
+
+    // Query the undo state, now that a "delete" is on the redo stack and an 
"insert" belongs to the
+    // view on the undo stack, so the types are different.
+    SwUndoId nUndoId(SwUndoId::EMPTY);
+    // Without the accompanying fix in place, this test would have failed with:
+    // runtime error: downcast which does not point to an object of type 
'const SwUndoInsert'
+    // note: object is of type 'SwUndoDelete'
+    // in an UBSan build.
+    pWrtShell1->GetLastUndoInfo(nullptr, &nUndoId, &pWrtShell1->GetView());
+}
+
 CPPUNIT_TEST_FIXTURE(SwTiledRenderingTest, testUndoReorderingMulti)
 {
     // Create two views and a document of 2 paragraphs.
diff --git a/sw/source/core/undo/docundo.cxx b/sw/source/core/undo/docundo.cxx
index 84fcd4fe5bb6..a9efc248259a 100644
--- a/sw/source/core/undo/docundo.cxx
+++ b/sw/source/core/undo/docundo.cxx
@@ -416,7 +416,7 @@ bool UndoManager::IsViewUndoActionIndependent(const SwView* 
pView, sal_uInt16& r
     for (size_t i = 0; i < GetRedoActionCount(); ++i)
     {
         auto pRedoAction = dynamic_cast<const SwUndo*>(GetRedoAction(i));
-        if (!pRedoAction || pViewSwAction->GetId() != SwUndoId::TYPING)
+        if (!pRedoAction || pRedoAction->GetId() != SwUndoId::TYPING)
         {
             return false;
         }
commit 4b73937c75be1c9338d90ee7616995bc1d521476
Author:     Miklos Vajna <vmik...@collabora.com>
AuthorDate: Tue May 23 15:04:22 2023 +0200
Commit:     Andras Timar <andras.ti...@collabora.com>
CommitDate: Thu May 25 09:07:43 2023 +0200

    tdf#155002 sw floattable, split but not yet moved follow: fix 
GetNextFlyLeaf()
    
    Trying to open the bugdoc resulted in a layout loop since commit
    b47401e12d9c45386899df0aa26653bd26c9abd4 (sw floattable: fix assert fail
    when object formatter gets the wrong anchor, 2023-05-22), previously
    could not open due to an assertion failure.
    
    What happened is that the anchor hosted a fly frame with a table, but
    the anchor was inside a section. Instead of splitting the fly and moving
    part of it to a next page, we constantly created and deleted new pages,
    without moving anything, which is a layout loop.
    
    In fact crating a new page in SwFrame::GetNextFlyLeaf() is not correct:
    we should realize that we're inside a section and let GetNextSctLeaf()
    handle the page (and section) creation. Once the anchor code is improved
    to detect that we're in a section, GetNextSctLeaf() creates the follow
    section, and later in GetNextFlyLeaf() the call to GetNextLayoutLeaf()
    will find the new follow section. Which means we have a place for the
    follow anchor and don't create any new page, fixing the loop.
    
    The import result of the bugdoc is still not perfect, but at least we
    don't crash or hang while opening the document.
    
    (cherry picked from commit 807ad65661c122a33fccb4fd3453ef92c0e9129d)
    
    Change-Id: Ic0717dddbcd0949df7d4ae766f3cc0dbbbcff27a
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/152188
    Reviewed-by: Andras Timar <andras.ti...@collabora.com>
    Tested-by: Andras Timar <andras.ti...@collabora.com>

diff --git a/sw/qa/core/layout/data/floattable-next-leaf-in-section.docx 
b/sw/qa/core/layout/data/floattable-next-leaf-in-section.docx
new file mode 100644
index 000000000000..3a315d51c984
Binary files /dev/null and 
b/sw/qa/core/layout/data/floattable-next-leaf-in-section.docx differ
diff --git a/sw/qa/core/layout/flycnt.cxx b/sw/qa/core/layout/flycnt.cxx
index 83889ceecef0..55ac7382a062 100644
--- a/sw/qa/core/layout/flycnt.cxx
+++ b/sw/qa/core/layout/flycnt.cxx
@@ -910,6 +910,17 @@ CPPUNIT_TEST_FIXTURE(Test, testSplitFlyObjectFormatter)
     CPPUNIT_ASSERT(pPage3);
     CPPUNIT_ASSERT(!pPage3->GetSortedObjs());
 }
+
+CPPUNIT_TEST_FIXTURE(Test, testSplitFlyNextLeafInSection)
+{
+    // Given a document with 4 pages: page 1 had a floating table, page 2 & 3 
had a second floating
+    // table and finally page 4 is empty:
+    createSwDoc("floattable-next-leaf-in-section.docx");
+
+    // When calculating the layout:
+    // Then this never returned, the loop in SwFrame::GetNextFlyLeaf() never 
finished.
+    calcLayout();
+}
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/layout/flycnt.cxx b/sw/source/core/layout/flycnt.cxx
index de26ee63a18a..0dbe031092dc 100644
--- a/sw/source/core/layout/flycnt.cxx
+++ b/sw/source/core/layout/flycnt.cxx
@@ -1555,6 +1555,20 @@ SwLayoutFrame *SwFrame::GetNextFlyLeaf( MakePageType 
eMakePage )
     assert(pFly->IsFlySplitAllowed() && "GetNextFlyLeaf: fly split not 
allowed");
 
     SwTextFrame* pFlyAnchor = pFly->FindAnchorCharFrame();
+
+    if (!pFlyAnchor)
+    {
+        // In case our fly frame is split already, but not yet moved, then 
FindAnchorCharFrame()
+        // won't find the anchor, since it wants a follow anchor, but there is 
no follow anchor yet.
+        // In this case work with a plain anchor, so FindSctFrame() works to 
find out we're in a
+        // section.
+        auto pAnchorFrame = const_cast<SwFrame*>(pFly->GetAnchorFrame());
+        if (pAnchorFrame && pAnchorFrame->IsTextFrame())
+        {
+            pFlyAnchor = static_cast<SwTextFrame*>(pAnchorFrame);
+        }
+    }
+
     bool bBody = pFlyAnchor && pFlyAnchor->IsInDocBody();
     SwLayoutFrame *pLayLeaf = nullptr;
     // Look up the first candidate.

Reply via email to