sw/qa/core/layout/data/floattable-hori-pos.docx |binary
sw/qa/core/layout/flycnt.cxx | 34 ++++++++++++++++++++++++
sw/qa/uitest/writer_tests6/tdf124675.py | 27 +++++++++----------
sw/source/core/text/frmform.cxx | 34 ++++++++++++++----------
uitest/uitest/test.py | 13 +++++++++
5 files changed, 81 insertions(+), 27 deletions(-)
New commits:
commit 3ef536a6755ffdcfc2657453444728cd0062d284
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Thu Mar 23 08:09:01 2023 +0100
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Wed Mar 29 10:17:08 2023 +0000
sw floattable: fix bad position of follow fly if anchor is positioned late
The bugdoc has a split fly, the master is centered horizontally. That
means the follow should be centered as well, but it was aligned to the
left.
The problem is that the follow fly was positioned when the anchor was
not positioned at all (its top left was 0,0) and then there was no
invalidation of the fly position to recalc it later when the anchor got
positioned. The existing InvalidatePos() call is not enough because once
a fly is formatted, its position gets locked and only
unlockPositionOfObjects() at the end of the layout process will unlock
it.
Fix the problem similar to what lcl_InvalidateLowerObjs() for SwTabFrame
does: if we know that the position of the anchor changed, then unlock
the position before invalidating it.
If this leads to unwanted re-positioning of flys, it would be perhaps
possible to do this when the anchor position changes from 0,0 to a valid
position, but for now just do this all time the anchor position changes.
(cherry picked from commit 12a9009a1c19ee26c65fb44fc90f3432c88ab6a5)
Change-Id: I811bdfa1eb6705ff3de6ec77111e9500617e8bee
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/149477
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
diff --git a/sw/qa/core/layout/data/floattable-hori-pos.docx
b/sw/qa/core/layout/data/floattable-hori-pos.docx
new file mode 100644
index 000000000000..7a5e033a4928
Binary files /dev/null and b/sw/qa/core/layout/data/floattable-hori-pos.docx
differ
diff --git a/sw/qa/core/layout/flycnt.cxx b/sw/qa/core/layout/flycnt.cxx
index 528b3bc4cbc8..b9ab0ac116cb 100644
--- a/sw/qa/core/layout/flycnt.cxx
+++ b/sw/qa/core/layout/flycnt.cxx
@@ -509,6 +509,40 @@ CPPUNIT_TEST_FIXTURE(Test, testSplitFlyCompat14Body)
SwFrame* pRow2 = pTab2->GetLower();
CPPUNIT_ASSERT(!pRow2->GetNext());
}
+
+CPPUNIT_TEST_FIXTURE(Test, testSplitFlyFollowHorizontalPosition)
+{
+ // Given a document with 2 pages, master fly on page 1, follow fly on page
2:
+ SwModelTestBase::FlySplitGuard aGuard;
+ createSwDoc("floattable-hori-pos.docx");
+
+ // When laying out that document:
+ calcLayout();
+
+ // Then make sure that the follow fly doesn't have a different horizontal
position:
+ SwDoc* pDoc = getSwDoc();
+ SwRootFrame* pLayout = pDoc->getIDocumentLayoutAccess().GetCurrentLayout();
+ auto pPage1 = dynamic_cast<SwPageFrame*>(pLayout->Lower());
+ CPPUNIT_ASSERT(pPage1);
+ const SwSortedObjs& rPage1Objs = *pPage1->GetSortedObjs();
+ CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), rPage1Objs.size());
+ auto pPage1Fly = dynamic_cast<SwFlyAtContentFrame*>(rPage1Objs[0]);
+ CPPUNIT_ASSERT(pPage1Fly);
+ tools::Long nPage1FlyLeft = pPage1Fly->getFrameArea().Left();
+ auto pPage2 = dynamic_cast<SwPageFrame*>(pPage1->GetNext());
+ CPPUNIT_ASSERT(pPage2);
+ const SwSortedObjs& rPage2Objs = *pPage2->GetSortedObjs();
+ CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), rPage2Objs.size());
+ auto pPage2Fly = dynamic_cast<SwFlyAtContentFrame*>(rPage2Objs[0]);
+ CPPUNIT_ASSERT(pPage2Fly);
+ tools::Long nPage2FlyLeft = pPage2Fly->getFrameArea().Left();
+ // Without the accompanying fix in place, this test would have failed with:
+ // - Expected: 5528
+ // - Actual : 284
+ // i.e. the follow fly was pushed towards the left, instead of having the
same position as the
+ // master fly.
+ CPPUNIT_ASSERT_EQUAL(nPage1FlyLeft, nPage2FlyLeft);
+}
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/text/frmform.cxx b/sw/source/core/text/frmform.cxx
index 83980f9c8b11..f756b186c464 100644
--- a/sw/source/core/text/frmform.cxx
+++ b/sw/source/core/text/frmform.cxx
@@ -340,25 +340,31 @@ bool SwTextFrame::CalcFollow(TextFrameIndex const
nTextOfst)
void SwTextFrame::MakePos()
{
+ Point aOldPos = getFrameArea().Pos();
SwFrame::MakePos();
- // Find the master frame.
- const SwTextFrame* pMaster = this;
- while (pMaster->IsFollow())
+ // Recalc split flys if our position changed.
+ if (aOldPos != getFrameArea().Pos())
{
- pMaster = pMaster->FindMaster();
- }
- // Find which flys are effectively anchored to this frame.
- for (const auto& pFly : pMaster->GetSplitFlyDrawObjs())
- {
- SwTextFrame* pFlyAnchor = pFly->FindAnchorCharFrame();
- if (pFlyAnchor != this)
+ // Find the master frame.
+ const SwTextFrame* pMaster = this;
+ while (pMaster->IsFollow())
+ {
+ pMaster = pMaster->FindMaster();
+ }
+ // Find which flys are effectively anchored to this frame.
+ for (const auto& pFly : pMaster->GetSplitFlyDrawObjs())
{
- continue;
+ SwTextFrame* pFlyAnchor = pFly->FindAnchorCharFrame();
+ if (pFlyAnchor != this)
+ {
+ continue;
+ }
+ // Possibly this fly was positioned relative to us, invalidate its
position now that our
+ // position is changed.
+ pFly->UnlockPosition();
+ pFly->InvalidatePos();
}
- // Possibly this fly was positioned relative to us, invalidate its
position now that our
- // position is changed.
- pFly->InvalidatePos();
}
// Inform LOK clients about change in position of redlines (if any)
commit 3ff9de9e06017375ea9abfc20b3eb0c7cd91c254
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Wed Mar 22 11:34:54 2023 +0100
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Wed Mar 29 10:17:00 2023 +0000
sw floattable: fix current page number when editing document with a split
fly
UITest_writer_tests6's
tdf124675.tdf124675.test_tdf124675_crash_moving_SwTextFrame_previous_page
failed in the SW_FORCE_FLY_SPLIT=1 case, since the current page after
typing was 3, not 2.
It seems this change is wanted, since the total page count increases by
2, so it's consistent that the current page increases similarly with
typing. Also, repeating the UITest in Word (after positioning the cursor
at the top paragraph in the document) also produces page 3 as the
current page.
Fix the problem by locally enabling split flys for this test and then we
can assert that we match Word. This requires a new context manager, but
that set_config() is really similar to other context managers in the
class that return no value.
The original purpose of the test was to make sure we don't crash,
anyway.
(cherry picked from commit 015da04a8f3e1368c6b9668ca22d7e320e1ecae6)
Change-Id: Id0dfde23a8726c8799950a6e4fdd1d85f135eafc
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/149376
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
diff --git a/sw/qa/uitest/writer_tests6/tdf124675.py
b/sw/qa/uitest/writer_tests6/tdf124675.py
index 40eb2987e260..93bae8ea9284 100644
--- a/sw/qa/uitest/writer_tests6/tdf124675.py
+++ b/sw/qa/uitest/writer_tests6/tdf124675.py
@@ -14,23 +14,24 @@ from uitest.uihelper.common import get_state_as_dict,
get_url_for_data_file
class tdf124675(UITestCase):
def test_tdf124675_crash_moving_SwTextFrame_previous_page(self):
- with self.ui_test.load_file(get_url_for_data_file("tdf124675.docx"))
as writer_doc:
- xWriterDoc = self.xUITest.getTopFocusWindow()
- xWriterEdit = xWriterDoc.getChild("writer_edit")
+ with
self.ui_test.set_config('/org.openoffice.Office.Writer/Filter/Import/DOCX/ImportFloatingTableAsSplitFly',
True):
+ with
self.ui_test.load_file(get_url_for_data_file("tdf124675.docx")) as writer_doc:
+ xWriterDoc = self.xUITest.getTopFocusWindow()
+ xWriterEdit = xWriterDoc.getChild("writer_edit")
- self.assertEqual(writer_doc.CurrentController.PageCount, 2)
- self.assertEqual(get_state_as_dict(xWriterEdit)["CurrentPage"],
"1")
+ self.assertEqual(writer_doc.CurrentController.PageCount, 2)
+
self.assertEqual(get_state_as_dict(xWriterEdit)["CurrentPage"], "1")
- for i in range(52):
- xWriterEdit.executeAction("TYPE", mkPropertyValues({"KEYCODE":
"RETURN"}))
+ for i in range(52):
+ xWriterEdit.executeAction("TYPE",
mkPropertyValues({"KEYCODE": "RETURN"}))
- self.assertEqual(writer_doc.CurrentController.PageCount, 4)
- self.assertEqual(get_state_as_dict(xWriterEdit)["CurrentPage"],
"2")
+ self.assertEqual(writer_doc.CurrentController.PageCount, 4)
+
self.assertEqual(get_state_as_dict(xWriterEdit)["CurrentPage"], "3")
- for i in range(52):
- self.xUITest.executeCommand(".uno:Undo")
+ for i in range(52):
+ self.xUITest.executeCommand(".uno:Undo")
- self.assertEqual(writer_doc.CurrentController.PageCount, 2)
- self.assertEqual(get_state_as_dict(xWriterEdit)["CurrentPage"],
"1")
+ self.assertEqual(writer_doc.CurrentController.PageCount, 2)
+
self.assertEqual(get_state_as_dict(xWriterEdit)["CurrentPage"], "1")
# vim: set shiftwidth=4 softtabstop=4 expandtab:
diff --git a/uitest/uitest/test.py b/uitest/uitest/test.py
index 5ed20add799c..5442d46b1d90 100644
--- a/uitest/uitest/test.py
+++ b/uitest/uitest/test.py
@@ -104,6 +104,19 @@ class UITest(object):
finally:
self.close_doc()
+ # Resets the setting to the old value at exit
+ @contextmanager
+ def set_config(self, path, new_value):
+ xChanges =
self._xContext.ServiceManager.createInstanceWithArgumentsAndContext('com.sun.star.configuration.ReadWriteAccess',
("",), self._xContext)
+ try:
+ old_value = xChanges.getByHierarchicalName(path)
+ xChanges.replaceByHierarchicalName(path, new_value)
+ xChanges.commitChanges()
+ yield
+ finally:
+ xChanges.replaceByHierarchicalName(path, old_value)
+ xChanges.commitChanges()
+
# Calls UITest.close_doc at exit
@contextmanager
def load_empty_file(self, app):
