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.