sw/qa/core/undo/data/anchor-type-change-position.docx |binary
sw/qa/core/undo/undo.cxx | 30 ++++++++++++++++++
sw/source/core/undo/unattr.cxx | 11 ++++++
3 files changed, 40 insertions(+), 1 deletion(-)
New commits:
commit 5a25d9252791409f5e73616ff752a9ae8227aaf7
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Mon Jun 5 20:07:56 2023 +0200
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Tue Jun 6 13:30:45 2023 +0200
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 <vmik...@collabora.com>
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
Binary files /dev/null and
b/sw/qa/core/undo/data/anchor-type-change-position.docx 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()));
}
