sw/qa/extras/ooxmlexport/data/tdf125469_singleSpacing.docx |binary
sw/qa/extras/ooxmlexport/ooxmlexport21.cxx | 36 +++++++++++++
sw/source/writerfilter/dmapper/DomainMapper.cxx | 36 +++++++++++--
sw/source/writerfilter/dmapper/DomainMapper_Impl.cxx | 13 ++++
sw/source/writerfilter/dmapper/DomainMapper_Impl.hxx | 10 +++
5 files changed, 91 insertions(+), 4 deletions(-)
New commits:
commit e3dfb54117f5325d8f38f58ef892c2808d601e54
Author: Justin Luth <jl...@mail.com>
AuthorDate: Wed Jul 17 14:48:57 2024 -0400
Commit: Justin Luth <jl...@mail.com>
CommitDate: Fri Jul 19 02:47:26 2024 +0200
tdf#125469 writerfilter: use "auto" for not-provided lineRule
This fixes the problem where a programmatically generated document
didn't specify a w:lineRule, and thus the w:line size
was treated as a FIXed spacing of 12pt instead of as single spacing.
------------------------------------------------------------
lineRule (Spacing Between Lines)
Specifies how the spacing between lines is calculated
as stored in the line attribute.
If this attribute is omitted, then it shall be assumed to be
of a value "auto" if a w:line attribute value is present.
------------------------------------------------------------
However, our implementation (wisely) defaulted to ::FIX
instead of ::PROP (which keeps precision in w:line sizes
since PROP is a integer percentage instead of a float
and thus loses precision when switching between FIX and PROP).
Note of course that if neither w:line nor w:lineRule is provided,
then we go with the LO default of mode=0, height=0 (aka AUTO).
This gets tricky because the lineRule can be inherited,
so just because it is missing in a pPr doesn't necessarily mean
we can just substitute "auto".
It is also tricky because w:spacing
deals with more than just line spacing.
Not surprisingly, no existing unit tests matched this case.
However, the unit tests were VERY helpful in pointing out
limitations as I attempted to code the fix.
make CppunitTest_sw_ooxmlexport21 \
CPPUNIT_TEST_NAME=testTdf125469_singleSpacing
Change-Id: Id5e95f2983f4fb435810d5a1f3fd25ded5118ddc
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170670
Tested-by: Jenkins
Reviewed-by: Justin Luth <jl...@mail.com>
diff --git a/sw/qa/extras/ooxmlexport/data/tdf125469_singleSpacing.docx
b/sw/qa/extras/ooxmlexport/data/tdf125469_singleSpacing.docx
new file mode 100644
index 000000000000..ff92e1b2d0c7
Binary files /dev/null and
b/sw/qa/extras/ooxmlexport/data/tdf125469_singleSpacing.docx differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport21.cxx
b/sw/qa/extras/ooxmlexport/ooxmlexport21.cxx
index 9ef569e47683..91fbf2e7e110 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport21.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport21.cxx
@@ -327,6 +327,42 @@ DECLARE_OOXMLEXPORT_TEST(testTdf158597, "tdf158597.docx")
}
}
+DECLARE_OOXMLEXPORT_TEST(testTdf125469_singleSpacing,
"tdf125469_singleSpacing.docx")
+{
+ // Given a document with 4 paragraphs of varying strange line spacing
definitions,
+ // and a DocDefault of single line spacing (AUTO 240pt) (240pt is 0.423 cm)
+
+ // Paragraph 1 - DocDefaults specifies size 240 without a lineRule -
default is AUTO(aka PROP)
+ // Visually, this should clearly say "Single spacing"
+ auto aSpacing = getProperty<style::LineSpacing>(getParagraph(1),
u"ParaLineSpacing"_ustr);
+ CPPUNIT_ASSERT_EQUAL(sal_Int16(style::LineSpacingMode::PROP),
aSpacing.Mode);
+ CPPUNIT_ASSERT_EQUAL(sal_Int16(100), aSpacing.Height);
+
+ // Paragraph 2 - paragraph style specifies atLeast 240, para overrides
with only -240.
+ // The negative value (always) turns the (inherited) "atLeast" into an
"exact".
+ // Visually, this is hardly readable (36pt font forced into 12pt space)
+ aSpacing = getProperty<style::LineSpacing>(getParagraph(2),
u"ParaLineSpacing"_ustr);
+ // CPPUNIT_ASSERT_EQUAL(sal_Int16(style::LineSpacingMode::FIX),
aSpacing.Mode);
+ // CPPUNIT_ASSERT_EQUAL(sal_Int16(423), aSpacing.Height);
+
+ // Paragraph 3 - paragraph style specifies exact 240, para overrides with
exact -240.
+ // The negative value turns the non-inherited "exact" into an "atLeast".
+ // Visually, this should clearly say "Negative exact"
+ aSpacing = getProperty<style::LineSpacing>(getParagraph(3),
u"ParaLineSpacing"_ustr);
+ // CPPUNIT_ASSERT_EQUAL(sal_Int16(style::LineSpacingMode::MINIMUM),
aSpacing.Mode);
+ // CPPUNIT_ASSERT_EQUAL(sal_Int16(423), aSpacing.Height);
+
+ // Paragraph 4 - paragraph style specifies exact 240, para overrides with
only -240.
+ // The negative value does nothing to the inherited "exact".
+ // Visually, this is hardly readable (36pt font forced into 12pt space)
+ aSpacing = getProperty<style::LineSpacing>(getParagraph(4),
u"ParaLineSpacing"_ustr);
+ // CPPUNIT_ASSERT_EQUAL(sal_Int16(style::LineSpacingMode::FIX),
aSpacing.Mode);
+ // CPPUNIT_ASSERT_EQUAL(sal_Int16(423), aSpacing.Height);
+
+ // all of this ends up being squeezed onto a single page
+ // CPPUNIT_ASSERT_EQUAL(1, getPages());
+}
+
DECLARE_OOXMLEXPORT_TEST(testTdf43767_caseMapNumbering,
"tdf43767_caseMapNumbering.odt")
{
// given a document with 2 numbered Lists [each entry restarts numbering
for visual comparison]
diff --git a/sw/source/writerfilter/dmapper/DomainMapper.cxx
b/sw/source/writerfilter/dmapper/DomainMapper.cxx
index 3bc6444ef15e..13ffe529d09d 100644
--- a/sw/source/writerfilter/dmapper/DomainMapper.cxx
+++ b/sw/source/writerfilter/dmapper/DomainMapper.cxx
@@ -508,6 +508,11 @@ void DomainMapper::lcl_attribute(Id nName, Value & val)
case NS_ooxml::LN_CT_Spacing_line: //91434
case NS_ooxml::LN_CT_Spacing_lineRule: //91435
{
+ if (nName == NS_ooxml::LN_CT_Spacing_line)
+ m_pImpl->SetSpacingHadLine(true);
+ else
+ m_pImpl->SetSpacingHadLineRule(true);
+
m_pImpl->SetLineSpacing(nName, nIntValue);
}
break;
@@ -2398,12 +2403,37 @@ void DomainMapper::sprmWithProps( Sprm& rSprm, const
PropertyMapPtr& rContext )
}
}
break;
+ case NS_ooxml::LN_CT_PPrBase_spacing:
+ {
+ m_pImpl->SetSpacingHadLine(false);
+ m_pImpl->SetSpacingHadLineRule(false);
+
+ resolveSprmProps(*this, rSprm);
+
+ // The implementation default is FIXED line spacing instead of the
documented default AUTO.
+ // If w:line seen, but no w:lineRule was provided, send appropriate
mode to finalize height.
+ if (m_pImpl->GetSpacingHadLine() && !m_pImpl->GetSpacingHadLineRule())
+ {
+ sal_Int32 nLineRule =
NS_ooxml::LN_Value_doc_ST_LineSpacingRule_auto; // default
+ // lineRule may have been provided via inherited paragraph style.
+ style::LineSpacing aInheritedSpacing;
+ m_pImpl->GetInheritedParaProperty(PROP_PARA_LINE_SPACING) >>=
aInheritedSpacing;
+ if (aInheritedSpacing.Mode == style::LineSpacingMode::FIX)
+ nLineRule = NS_ooxml::LN_Value_doc_ST_LineSpacingRule_exact;
+ else if (aInheritedSpacing.Mode == style::LineSpacingMode::MINIMUM)
+ nLineRule = NS_ooxml::LN_Value_doc_ST_LineSpacingRule_atLeast;
+
+ m_pImpl->SetLineSpacing(NS_ooxml::LN_CT_Spacing_lineRule,
nLineRule);
+ }
+
+ m_pImpl->appendGrabBag(m_pImpl->m_aInteropGrabBag, u"spacing"_ustr,
m_pImpl->m_aSubInteropGrabBag);
+ }
+ break;
case NS_ooxml::LN_CT_PPr_sectPr:
case NS_ooxml::LN_EG_RPrBase_rFonts:
case NS_ooxml::LN_EG_RPrBase_eastAsianLayout:
case NS_ooxml::LN_EG_RPrBase_u:
case NS_ooxml::LN_EG_RPrBase_lang:
- case NS_ooxml::LN_CT_PPrBase_spacing:
case NS_ooxml::LN_CT_PPrBase_ind:
case NS_ooxml::LN_CT_RPrDefault_rPr:
case NS_ooxml::LN_CT_PPrDefault_pPr:
@@ -2415,9 +2445,7 @@ void DomainMapper::sprmWithProps( Sprm& rSprm, const
PropertyMapPtr& rContext )
if (nSprmId == NS_ooxml::LN_CT_PPr_sectPr)
m_pImpl->SetParaSectpr(true);
resolveSprmProps(*this, rSprm);
- if (nSprmId == NS_ooxml::LN_CT_PPrBase_spacing)
- m_pImpl->appendGrabBag(m_pImpl->m_aInteropGrabBag,
u"spacing"_ustr, m_pImpl->m_aSubInteropGrabBag);
- else if (nSprmId == NS_ooxml::LN_EG_RPrBase_rFonts)
+ if (nSprmId == NS_ooxml::LN_EG_RPrBase_rFonts)
m_pImpl->appendGrabBag(m_pImpl->m_aInteropGrabBag, u"rFonts"_ustr,
m_pImpl->m_aSubInteropGrabBag);
else if (nSprmId == NS_ooxml::LN_EG_RPrBase_lang)
m_pImpl->appendGrabBag(m_pImpl->m_aInteropGrabBag, u"lang"_ustr,
m_pImpl->m_aSubInteropGrabBag);
diff --git a/sw/source/writerfilter/dmapper/DomainMapper_Impl.cxx
b/sw/source/writerfilter/dmapper/DomainMapper_Impl.cxx
index 79ae5e629263..df28343d3186 100644
--- a/sw/source/writerfilter/dmapper/DomainMapper_Impl.cxx
+++ b/sw/source/writerfilter/dmapper/DomainMapper_Impl.cxx
@@ -1536,6 +1536,19 @@ uno::Any
DomainMapper_Impl::GetPropertyFromParaStyleSheet(PropertyIds eId)
return GetPropertyFromStyleSheet(eId, pEntry, /*bDocDefaults=*/true,
/*bPara=*/true);
}
+uno::Any DomainMapper_Impl::GetInheritedParaProperty(PropertyIds eId)
+{
+ StyleSheetEntryPtr pEntry;
+ if ( m_bInStyleSheetImport )
+ pEntry = pEntry = GetStyleSheetTable()->FindStyleSheetByISTD(
+ GetStyleSheetTable()->GetCurrentEntry()->m_sBaseStyleIdentifier);
+ else
+ pEntry =
GetStyleSheetTable()->FindStyleSheetByConvertedStyleName(GetCurrentParaStyleName());
+
+ const bool bCheckDocDefaults = !IsDocDefaultsImport();
+ return GetPropertyFromStyleSheet(eId, pEntry, bCheckDocDefaults,
/*bPara=*/true);
+}
+
uno::Any DomainMapper_Impl::GetPropertyFromCharStyleSheet(PropertyIds eId,
const PropertyMapPtr& rContext)
{
if ( m_bInStyleSheetImport || eId == PROP_CHAR_STYLE_NAME ||
!isCharacterProperty(eId) )
diff --git a/sw/source/writerfilter/dmapper/DomainMapper_Impl.hxx
b/sw/source/writerfilter/dmapper/DomainMapper_Impl.hxx
index 5bb82fc715ae..e63c64778b09 100644
--- a/sw/source/writerfilter/dmapper/DomainMapper_Impl.hxx
+++ b/sw/source/writerfilter/dmapper/DomainMapper_Impl.hxx
@@ -666,6 +666,10 @@ private:
::std::set<::std::pair<PagePartType, PageType>> m_HeaderFooterSeen;
+ // LN_CT_PPrBase_spacing has specified both a line and a lineRule?
+ bool m_bSpacingHadLine = false;
+ bool m_bSpacingHadLineRule = false;
+
//annotation import
rtl::Reference< SwXTextField > m_xAnnotationField;
sal_Int32 m_nAnnotationId;
@@ -896,6 +900,8 @@ public:
css::uno::Any GetPropertyFromStyleSheet(PropertyIds eId,
StyleSheetEntryPtr pEntry, const bool bDocDefaults, const bool bPara, bool*
bIsDocDefault = nullptr);
// current paragraph style - including inherited properties
css::uno::Any GetPropertyFromParaStyleSheet(PropertyIds eId);
+ // only get inherited property
+ css::uno::Any GetInheritedParaProperty(PropertyIds eId);
// context's character style - including inherited properties
css::uno::Any GetPropertyFromCharStyleSheet(PropertyIds eId, const
PropertyMapPtr& rContext);
// get property first from the given context, or secondly via inheritance
from styles/docDefaults
@@ -1212,6 +1218,10 @@ public:
void SetParaAutoBefore(bool const bParaAutoBefore) {
m_StreamStateStack.top().bParaAutoBefore = bParaAutoBefore; }
+ bool GetSpacingHadLine() const { return m_bSpacingHadLine; }
+ void SetSpacingHadLine(bool bSet) { m_bSpacingHadLine = bSet; }
+ bool GetSpacingHadLineRule() const { return m_bSpacingHadLineRule; }
+ void SetSpacingHadLineRule(bool bSet) { m_bSpacingHadLineRule = bSet; }
void SetLineSpacing(const Id nName, sal_Int32 nIntValue);
/// Forget about the previous paragraph, as it's not inside the same
