summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMiklos Vajna <vmiklos@collabora.com>2023-06-05 20:07:56 +0200
committerMiklos Vajna <vmiklos@collabora.com>2023-06-06 13:30:45 +0200
commit5a25d9252791409f5e73616ff752a9ae8227aaf7 (patch)
tree4b85da1e889776ea470a585dd561e09fde09d792
parentf66d5331c1f55dfc88b1439f1b8e10ebe41d3f98 (diff)
tdf#140066 sw: fix bad position of textbox after anchor type change + undo
The bugdoc has a to-char anchored textbox. Changing the ancor type + undo resulted in a new textbox position, which is not expected. This is a problem since 7596e26fd259ce5445212949403e7cd32303b2bd (Add SwTextBoxHelper::findShapes, 2014-06-24), previously the codepath was not hit, since shape + content from DOCX was a fly format, not a draw format + fly format. Looking at the undo item implementation, in SwUndoFormatAttr::RestoreFlyAnchor(), aDrawSavePt is initialized from the old format (that we try to restore on undo), so in case that already matches aFormatPos (which is initialized from the new format / doc model), then adjusting the same position looks safe to skip, and that solves our problem as well. The whole problem area could be avoided if the draw layer would store absolute positions in its undo actions, because then it would be harder to get the abs Writer and the rel Draw undo actions out of sync, but that would a larger rework. Change-Id: Ia1c9f486e54f5967a34a1a269f8d234ae96d5bb6 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/152627 Tested-by: Jenkins Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
-rw-r--r--sw/qa/core/undo/data/anchor-type-change-position.docxbin0 -> 49986 bytes
-rw-r--r--sw/qa/core/undo/undo.cxx30
-rw-r--r--sw/source/core/undo/unattr.cxx11
3 files changed, 40 insertions, 1 deletions
diff --git a/sw/qa/core/undo/data/anchor-type-change-position.docx b/sw/qa/core/undo/data/anchor-type-change-position.docx
new file mode 100644
index 000000000000..1a5d27b3f4b8
--- /dev/null
+++ b/sw/qa/core/undo/data/anchor-type-change-position.docx
Binary files differ
diff --git a/sw/qa/core/undo/undo.cxx b/sw/qa/core/undo/undo.cxx
index 221f2ba4ece7..7b68e1b3b259 100644
--- a/sw/qa/core/undo/undo.cxx
+++ b/sw/qa/core/undo/undo.cxx
@@ -146,6 +146,36 @@ CPPUNIT_TEST_FIXTURE(SwCoreUndoTest, testImagePropsCreateUndoAndModifyDoc)
CPPUNIT_ASSERT(!pWrtShell->GetLastUndoInfo(nullptr, nullptr, nullptr));
}
+CPPUNIT_TEST_FIXTURE(SwCoreUndoTest, testAnchorTypeChangePosition)
+{
+ // Given a document with a textbox (draw + fly format pair) + an inner image:
+ createSwDoc("anchor-type-change-position.docx");
+ selectShape(1);
+ SwDoc* pDoc = getSwDoc();
+ const auto& rFormats = *pDoc->GetSpzFrameFormats();
+ CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(3), rFormats.size());
+ Point aOldPos;
+ {
+ const SwFormatHoriOrient& rHoriOrient = rFormats[0]->GetHoriOrient();
+ const SwFormatVertOrient& rVertOrient = rFormats[0]->GetVertOrient();
+ aOldPos = Point(rHoriOrient.GetPos(), rVertOrient.GetPos());
+ }
+
+ // When changing the anchor type + undo:
+ dispatchCommand(mxComponent, ".uno:SetAnchorToChar", {});
+ dispatchCommand(mxComponent, ".uno:Undo", {});
+
+ // Then make sure the old position is also restored:
+ const SwFormatHoriOrient& rHoriOrient = rFormats[0]->GetHoriOrient();
+ const SwFormatVertOrient& rVertOrient = rFormats[0]->GetVertOrient();
+ Point aNewPos(rHoriOrient.GetPos(), rVertOrient.GetPos());
+ // Without the accompanying fix in place, this test would have failed with:
+ // - Expected: 789,213
+ // - Actual : 1578,3425
+ // i.e. there was a big, unexpected increase in the vertical position after undo.
+ CPPUNIT_ASSERT_EQUAL(aOldPos, aNewPos);
+}
+
CPPUNIT_PLUGIN_IMPLEMENT();
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/undo/unattr.cxx b/sw/source/core/undo/unattr.cxx
index ef729671d936..9b6a12a43707 100644
--- a/sw/source/core/undo/unattr.cxx
+++ b/sw/source/core/undo/unattr.cxx
@@ -505,7 +505,16 @@ bool SwUndoFormatAttr::RestoreFlyAnchor(::sw::UndoRedoContext & rContext)
// The Draw model also prepared an Undo object for its right positioning
// which unfortunately is relative. Therefore block here a position
// change of the Contact object by setting the anchor.
- pFrameFormat->CallSwClientNotify(sw::RestoreFlyAnchorHint(aDrawSavePt));
+ const SwFormatVertOrient& rVertOrient = pFrameFormat->GetVertOrient();
+ const SwFormatHoriOrient& rHoriOrient = pFrameFormat->GetHoriOrient();
+ Point aFormatPos(rHoriOrient.GetPos(), rVertOrient.GetPos());
+ if (aDrawSavePt != aFormatPos)
+ {
+ // If the position would be the same, then skip the call: either it would do nothing or
+ // it would just go wrong.
+ pFrameFormat->CallSwClientNotify(sw::RestoreFlyAnchorHint(aDrawSavePt));
+ }
+
// cache the old value again
m_oOldSet->Put(SwFormatFrameSize(SwFrameSize::Variable, aDrawOldPt.X(), aDrawOldPt.Y()));
}