sw/qa/uibase/wrtsh/wrtsh.cxx      |   67 ++++++++++++++++++++++++++++++++++++++
 sw/source/core/inc/txtfrm.hxx     |    2 +
 sw/source/core/text/itratr.cxx    |    5 ++
 sw/source/uibase/wrtsh/delete.cxx |   23 +++++++++++++
 4 files changed, 97 insertions(+)

New commits:
commit 5b0256f30ee154edb28b999bc4faba2453fc32b8
Author:     Miklos Vajna <vmik...@collabora.com>
AuthorDate: Tue Sep 3 08:17:23 2024 +0200
Commit:     Miklos Vajna <vmik...@collabora.com>
CommitDate: Tue Sep 3 09:53:17 2024 +0200

    tdf#162507 sw floattable: fix not wanted join of two different split anchors
    
    Open the bugdoc, go to the end of the "Cannot press Delete here"
    paragraph, press delete, layout hangs.
    
    The trouble seems to be that this creates a doc model where multiple
    multi-page floating tables are anchored to the same paragraph, which is
    not something the layout supports. We explicitly avoid this at import
    time from DOCX since commit 01ad8ec4bb5425446e95dbada81de435646824b4 (sw
    floattable: fix lost tables around a floating table from DOCX,
    2023-06-05), but there was no mechanism to prevent the UI creating the
    same unsupported doc model.
    
    Fix the problem by extending SwWrtShell::DelRight() to prevent joining 2
    paragraphs in the unwanted case, interesting Word doesn't allow deleting
    at the end of that paragraph, either.
    
    Regression from commit 693ad3aadbf84afa750af73c330fe7e09b38a2e7
    (tdf#120262 sw floattable, legacy: go outside body only for page frame
    vert pos, 2023-07-31), which restricted the amount of cases where some
    weird legacy behavior was performed, so good to keep that side as-is.
    
    Change-Id: I4d8a6702d8ac1689690fa464014c99fcd5ef7bd2
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/172780
    Reviewed-by: Miklos Vajna <vmik...@collabora.com>
    Tested-by: Jenkins

diff --git a/sw/qa/uibase/wrtsh/wrtsh.cxx b/sw/qa/uibase/wrtsh/wrtsh.cxx
index 9c19c4474b9d..467068b69afc 100644
--- a/sw/qa/uibase/wrtsh/wrtsh.cxx
+++ b/sw/qa/uibase/wrtsh/wrtsh.cxx
@@ -28,6 +28,9 @@
 #include <textcontentcontrol.hxx>
 #include <fmtanchr.hxx>
 #include <view.hxx>
+#include <itabenum.hxx>
+#include <frmmgr.hxx>
+#include <formatflysplit.hxx>
 
 namespace
 {
@@ -451,6 +454,70 @@ CPPUNIT_TEST_FIXTURE(Test, 
testInsertComboBoxContentControl)
     std::shared_ptr<SwContentControl> pContentControl = 
rFormatContentControl.GetContentControl();
     CPPUNIT_ASSERT(pContentControl->GetComboBox());
 }
+
+void InsertSplitFly(SwWrtShell* pWrtShell)
+{
+    SwPosition aInsertPos = *pWrtShell->GetCursor()->GetPoint();
+    // Insert a table:
+    SwInsertTableOptions aTableOptions(SwInsertTableFlags::DefaultBorder, 0);
+    pWrtShell->InsertTable(aTableOptions, /*nRows=*/2, /*nCols=*/1);
+    pWrtShell->MoveTable(GotoPrevTable, fnTableStart);
+    pWrtShell->GoPrevCell();
+    pWrtShell->Insert(u"A1"_ustr);
+    // Select cell:
+    pWrtShell->SelAll();
+    // Select table:
+    pWrtShell->SelAll();
+    // Wrap the table in a text frame:
+    SwFlyFrameAttrMgr aMgr(true, pWrtShell, Frmmgr_Type::TEXT, nullptr);
+    pWrtShell->StartAllAction();
+    aMgr.InsertFlyFrame(RndStdIds::FLY_AT_PARA, aMgr.GetPos(), aMgr.GetSize());
+    pWrtShell->EndAllAction();
+    // Set fly properties:
+    pWrtShell->StartAllAction();
+    SwFrameFormat* pFly = pWrtShell->GetFlyFrameFormat();
+    SwAttrSet aSet(pFly->GetAttrSet());
+    aSet.Put(SwFormatFlySplit(true));
+    SwFormatAnchor aAnchor(RndStdIds::FLY_AT_PARA);
+    aAnchor.SetAnchor(&aInsertPos);
+    aSet.Put(aAnchor);
+    SwDoc* pDoc = pWrtShell->GetDoc();
+    pDoc->SetAttr(aSet, *pFly);
+    pWrtShell->EndAllAction();
+    pWrtShell->EnterStdMode();
+}
+
+CPPUNIT_TEST_FIXTURE(Test, testSplitFlysAnchorJoin)
+{
+    // Given a document with two paragraphs, each serving as an anchor of a 
split fly:
+    createSwDoc();
+    SwWrtShell* pWrtShell = getSwDocShell()->GetWrtShell();
+    pWrtShell->Insert(u"first para"_ustr);
+    pWrtShell->SplitNode();
+    pWrtShell->Insert(u"second para"_ustr);
+    pWrtShell->SttEndDoc(/*bStt=*/true);
+    InsertSplitFly(pWrtShell);
+    pWrtShell->SttEndDoc(/*bStt=*/false);
+    pWrtShell->SttPara();
+    InsertSplitFly(pWrtShell);
+
+    // When trying to delete at the end of the first para:
+    pWrtShell->SttEndDoc(/*bStt=*/true);
+    pWrtShell->EndPara();
+    pWrtShell->DelRight();
+
+    // Then make sure the join doesn't happen till a text node can only be an 
anchor for one split
+    // fly:
+    pWrtShell->SttEndDoc(/*bStt=*/true);
+    SwCursor* pCursor = pWrtShell->GetCursor();
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected: first para
+    // - Actual  : first parasecond para
+    // i.e. we did join the 2 anchors and for complex enough documents the 
layout never finished.
+    CPPUNIT_ASSERT_EQUAL(u"first para"_ustr, 
pCursor->GetPointNode().GetTextNode()->GetText());
+    pWrtShell->SttEndDoc(/*bStt=*/false);
+    CPPUNIT_ASSERT_EQUAL(u"second para"_ustr, 
pCursor->GetPointNode().GetTextNode()->GetText());
+}
 }
 
 CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/sw/source/core/inc/txtfrm.hxx b/sw/source/core/inc/txtfrm.hxx
index 186b014153c8..9cf16ac2bf10 100644
--- a/sw/source/core/inc/txtfrm.hxx
+++ b/sw/source/core/inc/txtfrm.hxx
@@ -807,6 +807,8 @@ public:
     /// with a negative vertical offset. Such frames may need explicit 
splitting.
     bool IsEmptyWithSplitFly() const;
 
+    bool HasSplitFlyDrawObjs() const;
+
     static SwView* GetView();
 
     void dumpAsXml(xmlTextWriterPtr writer = nullptr) const override;
diff --git a/sw/source/core/text/itratr.cxx b/sw/source/core/text/itratr.cxx
index 1c31831dd27f..b002e80610aa 100644
--- a/sw/source/core/text/itratr.cxx
+++ b/sw/source/core/text/itratr.cxx
@@ -1481,6 +1481,11 @@ std::vector<SwFlyAtContentFrame*> 
SwTextFrame::GetSplitFlyDrawObjs() const
     return aObjs;
 }
 
+bool SwTextFrame::HasSplitFlyDrawObjs() const
+{
+    return !GetSplitFlyDrawObjs().empty();
+}
+
 SwFlyAtContentFrame* SwTextFrame::HasNonLastSplitFlyDrawObj() const
 {
     const SwTextFrame* pFollow = GetFollow();
diff --git a/sw/source/uibase/wrtsh/delete.cxx 
b/sw/source/uibase/wrtsh/delete.cxx
index 4d0f857a45a5..2cb0e2ed6d72 100644
--- a/sw/source/uibase/wrtsh/delete.cxx
+++ b/sw/source/uibase/wrtsh/delete.cxx
@@ -36,6 +36,8 @@
 #include <osl/diagnose.h>
 #include <doc.hxx>
 #include <IDocumentRedlineAccess.hxx>
+#include <IDocumentLayoutAccess.hxx>
+#include <txtfrm.hxx>
 
 inline void SwWrtShell::OpenMark()
 {
@@ -336,6 +338,7 @@ bool SwWrtShell::DelRight(bool const isReplaceHeuristic)
             // #108049# Save the startnode of the current cell
             const SwStartNode* pSNdOld = pWasInTableNd ?
                 GetCursor()->GetPointNode().FindTableBoxStartNode() : nullptr;
+            SwTextNode* pOldTextNode = 
GetCursor()->GetPointNode().GetTextNode();
             bool bCheckDelFull = SelectionType::Text & nSelection && 
SwCursorShell::IsSttPara();
             bool bDelFull = false;
             bool bDoNothing = false;
@@ -365,6 +368,7 @@ bool SwWrtShell::DelRight(bool const isReplaceHeuristic)
             }
 
             // restore cursor
+            SwTextNode* pNewTextNode = 
GetCursor()->GetPointNode().GetTextNode();
             SwCursorShell::Pop(SwCursorShell::PopMode::DeleteCurrent);
 
             if (bDelFull)
@@ -372,6 +376,25 @@ bool SwWrtShell::DelRight(bool const isReplaceHeuristic)
                 DelFullPara();
                 UpdateAttr();
             }
+
+            if (pOldTextNode && pNewTextNode && pNewTextNode != pOldTextNode)
+            {
+                SwRootFrame* pLayout = 
mxDoc->getIDocumentLayoutAccess().GetCurrentLayout();
+                if (pLayout)
+                {
+                    auto pOldFrame = 
static_cast<SwTextFrame*>(pOldTextNode->getLayoutFrame(pLayout));
+                    auto pNewFrame = 
static_cast<SwTextFrame*>(pNewTextNode->getLayoutFrame(pLayout));
+                    if (pOldFrame && pNewFrame && 
pOldFrame->HasSplitFlyDrawObjs() && pNewFrame->HasSplitFlyDrawObjs())
+                    {
+                        // We have a selection where both the old and the new 
position is an anchor
+                        // for a potentially split fly. Don't allow join of 
the nodes in this case,
+                        // since the layout supports multiple anchors for one 
split fly, but it
+                        // doesn't support the usage of the same anchor for 
multiple split flys.
+                        bDoNothing = true;
+                    }
+                }
+            }
+
             if (bDelFull || bDoNothing)
                 break;
         }

Reply via email to