sw/qa/extras/ooxmlexport/data/tdf160077_layoutInCellC.docx |binary
sw/qa/extras/ooxmlexport/ooxmlexport21.cxx | 79 +++++++++----
sw/source/writerfilter/dmapper/GraphicImport.cxx | 31 ++---
3 files changed, 70 insertions(+), 40 deletions(-)
New commits:
commit 2c9f8ad2705834cdbe208373e10a0daad1040fd2
Author: Justin Luth <jl...@mail.com>
AuthorDate: Wed Aug 7 15:33:33 2024 -0400
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Thu Aug 8 14:06:14 2024 +0200
tdf#162211 revert "tdf#160077 writerfilter: shape vertRelation is FRAME
... for layoutinCell"
This reverts my 24.8 commit d210667c72ff821b8cb50b386a83ed55d65ae9bf
because I'm finding that PRINT_AREA is a better fit than FRAME.
Plus, add a few bits in addition to the revert.
Restrict killing layoutInCell to shapes
where the vertical offset is based on the paragraph too.
(If it is based on the page,
then treating as FRAME only works correctly
if the shape is anchored to the first paragraph.)
Also, just make clear this only makes sense to do
if layoutInCell is not already false, and if isInTable.
Existing unit tests affected were not interesting:
-tdf151704_thinColumnHeight.docx [two off-paper textboxes]
-tdf92157.docx [sync-separated textboxes, not seen in Word2010]
make CppunitTest_sw_ooxmlexport21 \
CPPUNIT_TEST_NAME=testTdf160077_layoutInCellC
Change-Id: I25e9a68f2bc096bddec6ba9d0dc1bb13a226999e
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171339
Tested-by: Jenkins
Reviewed-by: Justin Luth <jl...@mail.com>
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
diff --git a/sw/qa/extras/ooxmlexport/data/tdf160077_layoutInCellC.docx
b/sw/qa/extras/ooxmlexport/data/tdf160077_layoutInCellC.docx
new file mode 100644
index 000000000000..6a83893f76bb
Binary files /dev/null and
b/sw/qa/extras/ooxmlexport/data/tdf160077_layoutInCellC.docx differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport21.cxx
b/sw/qa/extras/ooxmlexport/ooxmlexport21.cxx
index 60ae2f1ea9ea..220641845563 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport21.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport21.cxx
@@ -517,26 +517,24 @@ DECLARE_OOXMLEXPORT_TEST(testTdf160077_layoutInCell,
"tdf160077_layoutInCell.doc
// (no top/bottom margins), but Cell A2 has a custom top margin of 2cm,
// so that effectively drops A1's print area down as well!
- xmlDocUniquePtr pDump = parseLayoutDump();
- const sal_Int32 nCellTop
- = getXPath(pDump, "//row[1]/cell[1]/infos/bounds"_ostr,
"top"_ostr).toInt32();
- const sal_Int32 nParaTop
- = getXPath(pDump, "//row[1]/cell[1]/txt/infos/bounds"_ostr,
"top"_ostr).toInt32();
- const sal_Int32 nImageTop
- = getXPath(pDump,
"//row[1]/cell[1]/txt/anchored/SwAnchoredDrawObject/bounds"_ostr,
- "top"_ostr)
- .toInt32();
- // The image is approximately half-way between cell top and the start of
the text
- // correct ImageTop: 3588, while incorrect value was 1117. Cell top is
3051, ParaTop is 4195
- const sal_Int32 nHalfway = nCellTop + (nParaTop - nCellTop) / 2;
- CPPUNIT_ASSERT_DOUBLES_EQUAL(nHalfway, nImageTop, 50); // +/- 4.4%
+ // xmlDocUniquePtr pDump = parseLayoutDump();
+ // const sal_Int32 nCellTop
+ // = getXPath(pDump, "//row[1]/cell[1]/infos/bounds"_ostr,
"top"_ostr).toInt32();
+ // const sal_Int32 nParaTop
+ // = getXPath(pDump, "//row[1]/cell[1]/txt/infos/bounds"_ostr,
"top"_ostr).toInt32();
+ // const sal_Int32 nImageTop
+ // = getXPath(pDump,
"//row[1]/cell[1]/txt/anchored/SwAnchoredDrawObject/bounds"_ostr,
+ // "top"_ostr)
+ // .toInt32();
+ // // The image is approximately half-way between cell top and the start
of the text
+ // // correct ImageTop: 3588, while incorrect value was 1117. Cell top is
3051, ParaTop is 4195
+ // const sal_Int32 nHalfway = nCellTop + (nParaTop - nCellTop) / 2;
+ // CPPUNIT_ASSERT_DOUBLES_EQUAL(nHalfway, nImageTop, 50); // +/- 4.4%
- // The effect is implemented by forcing "Entire paragraph area"/FRAME/0
- // instead of "Page text area"/PAGE_PRINT_AREA/8
- CPPUNIT_ASSERT_EQUAL(css::text::RelOrientation::FRAME,
+ CPPUNIT_ASSERT_EQUAL(css::text::RelOrientation::PAGE_PRINT_AREA,
getProperty<sal_Int16>(getShape(1),
u"VertOrientRelation"_ustr));
- // since layoutInCell had been turned off. If the implementation changes,
check the layout.
- CPPUNIT_ASSERT(!getProperty<bool>(getShape(1),
u"IsFollowingTextFlow"_ustr));
+
+ CPPUNIT_ASSERT(getProperty<bool>(getShape(1),
u"IsFollowingTextFlow"_ustr));
}
DECLARE_OOXMLEXPORT_TEST(testTdf160077_layoutInCellB,
"tdf160077_layoutInCellB.docx")
@@ -547,15 +545,48 @@ DECLARE_OOXMLEXPORT_TEST(testTdf160077_layoutInCellB,
"tdf160077_layoutInCellB.d
// This unit test is virtually the same idea as the previous one, with the
main benefit being
// that it causes an NS_ooxml::LN_Shape exception.
+ // xmlDocUniquePtr pDump = parseLayoutDump();
+ // const sal_Int32 nShapeTop
+ // = getXPath(pDump,
+ //
"//body/tab[1]/row[1]/cell[1]/txt[1]/anchored/SwAnchoredDrawObject/bounds"_ostr,
+ // "top"_ostr)
+ // .toInt32();
+ // // The shape is approximately 1 cm below the top of the page, and
~0.5cm above the cell
+ // // correct ShapeTop: 888 TWIPS, while incorrect value was -480. Cell
top is 1148, PageTop is 284
+ // CPPUNIT_ASSERT_DOUBLES_EQUAL(888, nShapeTop, 50);
+
+ const auto& xShape = getShapeByName(u"Group 1");
+ CPPUNIT_ASSERT_EQUAL(css::text::RelOrientation::PAGE_FRAME,
+ getProperty<sal_Int16>(xShape,
u"VertOrientRelation"_ustr));
+
+ CPPUNIT_ASSERT(getProperty<bool>(xShape, u"IsFollowingTextFlow"_ustr));
+}
+
+DECLARE_OOXMLEXPORT_TEST(testTdf160077_layoutInCellC,
"tdf160077_layoutInCellC.docx")
+{
+ // given an in-table, slightly rotated image vertically aligned to top
page margin
+ // (which is actually forced to layoutInCell, so that becomes the top of
the cell text area).
+ // This test anchors the image on paragraph 5 - proving vertical cannot
change to FRAME.
+
xmlDocUniquePtr pDump = parseLayoutDump();
- const sal_Int32 nShapeTop
- = getXPath(pDump,
-
"//body/tab[1]/row[1]/cell[1]/txt[1]/anchored/SwAnchoredDrawObject/bounds"_ostr,
+ const sal_Int32 nCellTop
+ = getXPath(pDump, "//row[1]/cell[2]/infos/bounds"_ostr,
"top"_ostr).toInt32();
+ const sal_Int32 nPara1Top
+ = getXPath(pDump, "//row[1]/cell[2]/txt[1]/infos/bounds"_ostr,
"top"_ostr).toInt32();
+ const sal_Int32 nImageTop
+ = getXPath(pDump,
"//row[1]/cell[2]/txt[5]/anchored/SwAnchoredDrawObject/bounds"_ostr,
"top"_ostr)
.toInt32();
- // The shape is approximately 1 cm below the top of the page, and ~0.5cm
above the cell
- // correct ShapeTop: 888 TWIPS, while incorrect value was -480. Cell top
is 1148, PageTop is 284
- CPPUNIT_ASSERT_DOUBLES_EQUAL(888, nShapeTop, 50);
+ // The image's top should be positioned at the start of the cell's text
area (i.e. para1 top)
+ // Before the revert, the image was positioned at the top of para 5.
+ CPPUNIT_ASSERT_LESS(nPara1Top, nImageTop); // Image not lower than para 1
+ // The image must be limited to the top of the cell, not the page
+ CPPUNIT_ASSERT_GREATER(nCellTop, nImageTop); // Image is lower (greater)
than top of cell
+
+ CPPUNIT_ASSERT_EQUAL(css::text::RelOrientation::PAGE_FRAME,
+ getProperty<sal_Int16>(getShape(1),
u"VertOrientRelation"_ustr));
+ // LayoutInCell must be enforced, to keep the image inside the cell
boundaries
+ CPPUNIT_ASSERT(getProperty<bool>(getShape(1),
u"IsFollowingTextFlow"_ustr));
}
DECLARE_OOXMLEXPORT_TEST(testTdf153909_followTextFlow,
"tdf153909_followTextFlow.docx")
diff --git a/sw/source/writerfilter/dmapper/GraphicImport.cxx
b/sw/source/writerfilter/dmapper/GraphicImport.cxx
index d516d59c2120..e32189643fb7 100644
--- a/sw/source/writerfilter/dmapper/GraphicImport.cxx
+++ b/sw/source/writerfilter/dmapper/GraphicImport.cxx
@@ -1283,24 +1283,16 @@ void GraphicImport::lcl_attribute(Id nName, Value&
rValue)
// But they aren't Writer pictures, either (which are
already handled above).
uno::Reference< beans::XPropertySet >
xShapeProps(m_xShape, uno::UNO_QUERY_THROW);
- if (m_pImpl->m_nWrap == text::WrapTextMode_THROUGH &&
m_pImpl->m_nHoriRelation == text::RelOrientation::FRAME)
+ if (m_pImpl->m_nWrap == text::WrapTextMode_THROUGH
+ && m_pImpl->m_bLayoutInCell &&
m_pImpl->m_rDomainMapper.IsInTable()
+ && m_pImpl->m_nHoriRelation ==
text::RelOrientation::FRAME
+ && m_pImpl->m_nVertRelation ==
text::RelOrientation::FRAME)
{
- if (m_pImpl->m_bLayoutInCell &&
m_pImpl->m_rDomainMapper.IsInTable()
- && (m_pImpl->m_nVertRelation ==
text::RelOrientation::PAGE_FRAME
- || m_pImpl->m_nVertRelation ==
text::RelOrientation::PAGE_PRINT_AREA))
- {
- // Impossible to be page-oriented when layout
in cell.
- // Since we are turning LayoutInCell off (to
simplify layout),
- // we need to set the orientation to the
paragraph,
- // as MSO effectively does when it forces
layoutInCell.
- // Probably also needs to happen with
TEXT_LINE,
- // but MSO is really weird with vertical
relation to "line"
- m_pImpl->m_nVertRelation =
text::RelOrientation::FRAME;
- }
-
- // text::RelOrientation::FRAME is OOXML's
"column", which behaves as if
- // layout-in-cell would be always off.
+ // wrapThrough paragraph-positioned shape is not
constrained by the cell
m_pImpl->m_bLayoutInCell = false;
+ // TODO: But we should not go in this direction.
MSO in compat15
+ // ignores layoutInCell, always treating it as
TRUE, not false,
+ // which is the opposite of what is happening here.
}
if (m_pImpl->m_nHoriRelation ==
text::RelOrientation::FRAME
@@ -1349,7 +1341,14 @@ void GraphicImport::lcl_attribute(Id nName, Value&
rValue)
if (m_pImpl->m_bLayoutInCell && bTextBox &&
m_pImpl->m_rDomainMapper.IsInTable()
&& m_pImpl->m_nHoriRelation ==
text::RelOrientation::PAGE_FRAME)
+ {
+ // This claim is a bit of a stretch, but should be
OK for horizontal.
+ // For layoutInCell, MSO does apply PAGE_FRAME and
PAGE_PRINT_AREA
+ // horiontal adjustments onto the cell's frame and
print area.
+ // Note that FRAME cannot be subsituted for
vertical (only first para).
m_pImpl->m_nHoriRelation =
text::RelOrientation::FRAME;
+ }
+
if(m_pImpl->m_rDomainMapper.IsInTable())
xShapeProps->setPropertyValue(getPropertyName(PROP_FOLLOW_TEXT_FLOW),
uno::Any(m_pImpl->m_bLayoutInCell));
