diff options
author | Miklos Vajna <vmiklos@collabora.com> | 2023-06-05 20:07:56 +0200 |
---|---|---|
committer | Miklos Vajna <vmiklos@collabora.com> | 2023-06-06 13:30:45 +0200 |
commit | 5a25d9252791409f5e73616ff752a9ae8227aaf7 (patch) | |
tree | 4b85da1e889776ea470a585dd561e09fde09d792 | |
parent | f66d5331c1f55dfc88b1439f1b8e10ebe41d3f98 (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.docx | bin | 0 -> 49986 bytes | |||
-rw-r--r-- | sw/qa/core/undo/undo.cxx | 30 | ||||
-rw-r--r-- | sw/source/core/undo/unattr.cxx | 11 |
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 Binary files differnew file mode 100644 index 000000000000..1a5d27b3f4b8 --- /dev/null +++ b/sw/qa/core/undo/data/anchor-type-change-position.docx 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())); } |