[Libreoffice-commits] core.git: include/oox oox/source sw/qa sw/source writerfilter/source
Justin Luth (via logerrit)
logerrit at kemper.freedesktop.org
Fri Nov 22 09:48:29 UTC 2019
include/oox/vml/vmltextbox.hxx | 1
oox/source/vml/vmltextbox.cxx | 34 ++++++++++++
oox/source/vml/vmltextboxcontext.cxx | 3 +
sw/qa/extras/ooxmlexport/ooxmlexport10.cxx | 8 ++
sw/qa/extras/ooxmlexport/ooxmlexport8.cxx | 13 ++++
sw/qa/extras/ooxmlimport/ooxmlimport.cxx | 5 +
sw/source/filter/ww8/docxattributeoutput.cxx | 22 --------
writerfilter/source/dmapper/DomainMapper_Impl.cxx | 59 ++++++++++++++++++++++
writerfilter/source/dmapper/StyleSheetTable.cxx | 14 -----
9 files changed, 121 insertions(+), 38 deletions(-)
New commits:
commit 334409fbde555a957cd34e295cc27f2c2bf6e194
Author: Justin Luth <justin.luth at collabora.com>
AuthorDate: Tue Oct 22 15:50:19 2019 +0300
Commit: Miklos Vajna <vmiklos at collabora.com>
CommitDate: Fri Nov 22 10:47:44 2019 +0100
tdf#128153 docx/VML: apply style properties to shape text
This replaces LO 4.3 commit 8766011bccfd0f12f8dd77d2f94eb16e2e8c3471
DOCX import: set document-level font size default based on default para style
...and is needed for tdf#118947, since bogus DEFAULT_VALUEs
really hinder determining what a table style should override.
Shape text should inherit the font size from the style that is specified.
In many cases, it will not be specified, and therefore the default style
was appropriate, but in cases where a style IS specified, then of course
use that fontsize ... and every other character and paragraph property.
HOWEVER, I only added the properties used in vml import for now,
and I skipped asian/complex fontnames since VML only handles CharHeight,
and not CharHeightAsian/Complex.
-note: this does not affect direct formatting - it just sets
default value at the shape level, not at the paragraph level.
So far I have only looked at DOCX:VML - which satisfies the
unit tests. There are other codepaths that lead to PushShapeContext
though, and this should be easy to expand to other import
situations. I've tried lots of asserts to find unit tests
that should be modified, and so far they all seemed to point
to VML - although round-tripping doesn't use VML and still
requires at minimum the CharHeight property to be overridden,
so limiting non-VML to that to maintain backward compatibility,
and reduce regression footprint.
Since we have to emulate here (since paragraph styles are not
supported on Draw text), a perfect solution cannot possibly be
found - specifically in cases where multiple paragraphs exist
in one shape with different styles applied, or where some
pararaphs apply a paragraph property and others do not.
Compromise 1: For ambiguous paragraph styles, fallback to the
default paragraph style. Rationale: Normally, most styles
inherit from default and only change a couple of properties.
So MOST of the properties will match the normal style.
The chances that the default style will be correct are
more likely than that some other random default would be.
-note: no existing unit tests were ambiguous
Compromise 2: Ideally, each paragraph could report whether
it had DIRECT formatting, and the paragraphs could be
walked through instead of the shapes. But I don't think
that is reasonably possible in this SyncProperties situation.
At first I had a grabbag framework setup that monitored
when a paragraph property was set, and then skipped applying
that property. But I later noticed that the PropertyState
for paragraph properties actually did seem to reflect
that - which is a better solution if it works properly.
Regression potential:
-for VML: should be limited to non-charheight properties
where the default style sets some weird default, and
an ambiguous style sets it back to the program default.
Prior to this patch, the default style value wouldn't apply.
On the flip side (and more likely scenario), non-ambiguous
cases will use the correct value and look more like MSWord,
as seen in many existing unit tests that now use corrected fonts.
-for non-VML: should be none since I limit it to only
CharHeight which was previously emulated by changing the
program default.
Change-Id: I8f1fb7ed01f990dbf998ebe04064c2645a68e1aa
Reviewed-on: https://gerrit.libreoffice.org/81365
Tested-by: Jenkins
Reviewed-by: Justin Luth <justin_luth at sil.org>
Reviewed-by: Miklos Vajna <vmiklos at collabora.com>
diff --git a/include/oox/vml/vmltextbox.hxx b/include/oox/vml/vmltextbox.hxx
index e361d1b7040c..7a37577c71ad 100644
--- a/include/oox/vml/vmltextbox.hxx
+++ b/include/oox/vml/vmltextbox.hxx
@@ -43,6 +43,7 @@ struct ShapeTypeModel;
struct TextParagraphModel
{
OptValue<OUString> moParaAdjust; ///< Paragraph adjust (left, center, right, etc.)
+ OptValue<OUString> moParaStyleName;
};
/** Font settings for a text portion in a textbox. */
diff --git a/oox/source/vml/vmltextbox.cxx b/oox/source/vml/vmltextbox.cxx
index ec56cb049fb2..c170c8db4414 100644
--- a/oox/source/vml/vmltextbox.cxx
+++ b/oox/source/vml/vmltextbox.cxx
@@ -28,6 +28,7 @@
#include <com/sun/star/text/WritingMode.hpp>
#include <com/sun/star/style/ParagraphAdjust.hpp>
#include <comphelper/sequence.hxx>
+#include <comphelper/sequenceashashmap.hxx>
namespace oox {
namespace vml {
@@ -76,6 +77,9 @@ OUString TextBox::getText() const
void TextBox::convert(const uno::Reference<drawing::XShape>& xShape) const
{
uno::Reference<text::XTextAppend> xTextAppend(xShape, uno::UNO_QUERY);
+ OUString sParaStyle;
+ bool bAmbiguousStyle = true;
+
for (auto const& portion : maPortions)
{
beans::PropertyValue aPropertyValue;
@@ -129,6 +133,25 @@ void TextBox::convert(const uno::Reference<drawing::XShape>& xShape) const
aPropertyValue.Value <<= eAdjust;
aPropVec.push_back(aPropertyValue);
}
+
+ // All paragraphs should be either undefined (default) or the same style,
+ // because it will only be applied to the entire shape, and not per-paragraph.
+ if (sParaStyle.isEmpty() )
+ {
+ if ( rParagraph.moParaStyleName.has() )
+ sParaStyle = rParagraph.moParaStyleName.get();
+ if ( bAmbiguousStyle )
+ bAmbiguousStyle = false; // both empty parastyle and ambiguous can only be true at the first paragraph
+ else
+ bAmbiguousStyle = rParagraph.moParaStyleName.has(); // ambiguous if both default and specified style used.
+ }
+ else if ( !bAmbiguousStyle )
+ {
+ if ( !rParagraph.moParaStyleName.has() )
+ bAmbiguousStyle = true; // ambiguous if both specified and default style used.
+ else if ( rParagraph.moParaStyleName.get() != sParaStyle )
+ bAmbiguousStyle = true; // ambiguous if two different styles specified.
+ }
if (rFont.moColor.has())
{
aPropertyValue.Name = "CharColor";
@@ -138,6 +161,17 @@ void TextBox::convert(const uno::Reference<drawing::XShape>& xShape) const
xTextAppend->appendTextPortion(portion.maText, comphelper::containerToSequence(aPropVec));
}
+ try
+ {
+ // Track the style in a grabBag for use later when style details are known.
+ comphelper::SequenceAsHashMap aGrabBag;
+ uno::Reference<beans::XPropertySet> xPropertySet(xShape, uno::UNO_QUERY_THROW);
+ aGrabBag.update( xPropertySet->getPropertyValue("CharInteropGrabBag") );
+ aGrabBag["mso-pStyle"] <<= sParaStyle;
+ xPropertySet->setPropertyValue("CharInteropGrabBag", uno::makeAny(aGrabBag.getAsConstPropertyValueList()));
+ }
+ catch (uno::Exception&) {}
+
// Remove the last character of the shape text, if it would be a newline.
uno::Reference< text::XTextCursor > xCursor = xTextAppend->createTextCursor();
xCursor->gotoEnd(false);
diff --git a/oox/source/vml/vmltextboxcontext.cxx b/oox/source/vml/vmltextboxcontext.cxx
index 7f5072536983..5f84ca33c2c6 100644
--- a/oox/source/vml/vmltextboxcontext.cxx
+++ b/oox/source/vml/vmltextboxcontext.cxx
@@ -270,6 +270,9 @@ void TextBoxContext::onStartElement(const AttributeList& rAttribs)
case W_TOKEN(jc):
maParagraph.moParaAdjust = rAttribs.getString( W_TOKEN(val) );
break;
+ case W_TOKEN(pStyle):
+ maParagraph.moParaStyleName = rAttribs.getString( W_TOKEN(val) );
+ break;
}
}
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx
index b9efb409a456..dada93a93b0f 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx
@@ -565,6 +565,14 @@ DECLARE_OOXMLEXPORT_TEST(testFdo74401, "fdo74401.docx")
uno::Reference<drawing::XShapeDescriptor> xShape(xGroupShape->getByIndex(1), uno::UNO_QUERY);
// The triangle (second child) was a TextShape before, so it was shown as a rectangle.
CPPUNIT_ASSERT_EQUAL(OUString("com.sun.star.drawing.CustomShape"), xShape->getShapeType());
+
+ uno::Reference<text::XText> xText = uno::Reference<text::XTextRange>(xShape, uno::UNO_QUERY_THROW)->getText();
+ uno::Reference<text::XTextRange> xCharRun = getRun(getParagraphOfText(1, xText), 1, "Triangle ");
+
+ // tdf#128153 Paragraph Style Normal (Web) should not overwrite the 11pt directly applied fontsize.
+ CPPUNIT_ASSERT_EQUAL_MESSAGE("Fontsize", 11.f, getProperty<float>(xCharRun, "CharHeight"));
+ // but paragraph Style Normal (Web) should provide the font name
+ CPPUNIT_ASSERT_EQUAL_MESSAGE("Font", OUString("Times New Roman"), getProperty<OUString>(xCharRun, "CharFontName"));
}
DECLARE_OOXMLEXPORT_TEST(testGridBefore, "gridbefore.docx")
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport8.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport8.cxx
index e5b4ffc9cfdd..537e6d5a148b 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport8.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport8.cxx
@@ -1028,9 +1028,18 @@ DECLARE_OOXMLEXPORT_TEST(testFdo46361, "fdo46361.docx")
// This was black, not green.
CPPUNIT_ASSERT_EQUAL(sal_Int32(0x008000), getProperty<sal_Int32>(getRun(xParagraph, 1), "CharColor"));
// \n char was missing due to unhandled w:br.
- CPPUNIT_ASSERT_EQUAL(OUString("text\ntext"), uno::Reference<text::XTextRange>(xGroupShape->getByIndex(1), uno::UNO_QUERY_THROW)->getString());
+ uno::Reference<text::XText> xShapeText(xGroupShape->getByIndex(1), uno::UNO_QUERY_THROW);
+ CPPUNIT_ASSERT_EQUAL(OUString("text\ntext"), xShapeText->getString());
// \n chars were missing, due to unhandled multiple w:p tags.
- CPPUNIT_ASSERT_EQUAL(OUString("text\ntext\n"), uno::Reference<text::XTextRange>(xGroupShape->getByIndex(2), uno::UNO_QUERY_THROW)->getString());
+ xShapeText.set(xGroupShape->getByIndex(2), uno::UNO_QUERY_THROW);
+ CPPUNIT_ASSERT_EQUAL(OUString("text\ntext\n"), xShapeText->getString());
+
+ // tdf#128153 The first and second lines are directly specified as centered. Make sure that doesn't change.
+ xParagraph.set(getParagraphOfText(2, xShapeText->getText(), "text"));
+ CPPUNIT_ASSERT_EQUAL(style::ParagraphAdjust_CENTER, static_cast<style::ParagraphAdjust>(getProperty<sal_Int16>(xParagraph, "ParaAdjust")));
+ // The last paragraph should be left aligned.
+ xParagraph.set(getParagraphOfText(3, xShapeText->getText(), ""));
+ CPPUNIT_ASSERT_MESSAGE("You FIXED me!", style::ParagraphAdjust_LEFT != static_cast<style::ParagraphAdjust>(getProperty<sal_Int16>(xParagraph, "ParaAdjust")));
}
DECLARE_OOXMLEXPORT_TEST(testFdo65632, "fdo65632.docx")
diff --git a/sw/qa/extras/ooxmlimport/ooxmlimport.cxx b/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
index 26b0b56eacae..2bc6a526fd99 100644
--- a/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
+++ b/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
@@ -569,7 +569,10 @@ DECLARE_OOXMLIMPORT_TEST(testGroupshapeChildRotation, "groupshape-child-rotation
#if HAVE_MORE_FONTS
xShape.set(xGroupShape->getByIndex(4), uno::UNO_QUERY);
// This was 887, i.e. border distances were included in the height.
- CPPUNIT_ASSERT_EQUAL(sal_Int32(686), xShape->getSize().Height);
+ CPPUNIT_ASSERT_EQUAL(sal_Int32(661), xShape->getSize().Height);
+ // Paragraph Style Normal should provide the font name - which slightly affects the shape's height (was 686)
+ uno::Reference<text::XText> xText = uno::Reference<text::XTextRange>(xShape, uno::UNO_QUERY_THROW)->getText();
+ CPPUNIT_ASSERT_EQUAL_MESSAGE("Font", OUString("Times New Roman"), getProperty<OUString>(getRun(xText, 1), "CharFontName"));
#endif
uno::Reference<drawing::XShapeDescriptor> xShapeDescriptor(xGroupShape->getByIndex(5), uno::UNO_QUERY);
diff --git a/sw/source/filter/ww8/docxattributeoutput.cxx b/sw/source/filter/ww8/docxattributeoutput.cxx
index 4f18582b0d7f..b6caf17412b3 100644
--- a/sw/source/filter/ww8/docxattributeoutput.cxx
+++ b/sw/source/filter/ww8/docxattributeoutput.cxx
@@ -4440,26 +4440,6 @@ void DocxAttributeOutput::LatentStyles()
m_pSerializer->endElementNS(XML_w, XML_latentStyles);
}
-namespace
-{
-
-/// Should the font size we have written out as a default one?
-bool lcl_isDefaultFontSize(const SvxFontHeightItem& rFontHeight, SwDoc* pDoc)
-{
- bool bRet = rFontHeight.GetHeight() != 200; // see StyleSheetTable_Impl::StyleSheetTable_Impl() where we set this default
- // Additionally, if the default para style has the same font size, then don't write it here.
- SwTextFormatColl* pDefaultStyle = pDoc->getIDocumentStylePoolAccess().GetTextCollFromPool(RES_POOLCOLL_STANDARD);
- if (pDefaultStyle)
- {
- const SfxPoolItem* pItem = nullptr;
- if (pDefaultStyle->GetAttrSet().HasItem(RES_CHRATR_FONTSIZE, &pItem))
- return static_cast<const SvxFontHeightItem*>(pItem)->GetHeight() != rFontHeight.GetHeight();
- }
- return bRet;
-}
-
-}
-
void DocxAttributeOutput::OutputDefaultItem(const SfxPoolItem& rHt)
{
bool bMustWrite = true;
@@ -4484,7 +4464,7 @@ void DocxAttributeOutput::OutputDefaultItem(const SfxPoolItem& rHt)
bMustWrite = true;
break;
case RES_CHRATR_FONTSIZE:
- bMustWrite = lcl_isDefaultFontSize(static_cast< const SvxFontHeightItem& >(rHt), m_rExport.m_pDoc);
+ bMustWrite = static_cast< const SvxFontHeightItem& >(rHt).GetHeight() != 200; // see StyleSheetTable_Impl::StyleSheetTable_Impl() where we set this default
break;
case RES_CHRATR_KERNING:
bMustWrite = static_cast< const SvxKerningItem& >(rHt).GetValue() != 0;
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index 41612fefe9b8..db838493e60e 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -2574,6 +2574,65 @@ void DomainMapper_Impl::PushShapeContext( const uno::Reference< drawing::XShape
uno::Reference< lang::XServiceInfo > xSInfo( xShape, uno::UNO_QUERY_THROW );
if (xSInfo->supportsService("com.sun.star.drawing.GroupShape"))
{
+ // Textboxes in shapes do not support styles, so check saved style information and apply properties directly to the child shapes.
+ const uno::Reference<drawing::XShapes> xShapes(xShape, uno::UNO_QUERY);
+ const sal_uInt32 nShapeCount = xShapes.is() ? xShapes->getCount() : 0;
+ for ( sal_uInt32 i = 0; i < nShapeCount; ++i )
+ {
+ try
+ {
+ uno::Reference<beans::XPropertySet> xSyncedPropertySet(xShapes->getByIndex(i), uno::UNO_QUERY_THROW);
+ comphelper::SequenceAsHashMap aGrabBag( xSyncedPropertySet->getPropertyValue("CharInteropGrabBag") );
+
+ // only VML import has checked for style. Don't apply default parastyle properties to other imported shapes
+ // - except for fontsize - to maintain compatibility with previous versions of LibreOffice.
+ const bool bOnlyApplyCharHeight = !aGrabBag["mso-pStyle"].hasValue();
+
+ OUString sStyleName;
+ aGrabBag["mso-pStyle"] >>= sStyleName;
+ StyleSheetEntryPtr pEntry = GetStyleSheetTable()->FindStyleSheetByISTD( sStyleName );
+ if ( !pEntry )
+ {
+ // Use default style even in ambiguous cases (where multiple styles were defined) since MOST styles inherit
+ // MOST of their properties from the default style. In the ambiguous case, we have to accept some kind of compromise
+ // and the default paragraph style ought to be the safest one... (compared to DocDefaults or program defaults)
+ pEntry = GetStyleSheetTable()->FindStyleSheetByConvertedStyleName( GetDefaultParaStyleName() );
+ }
+ if ( pEntry )
+ {
+ // The Ids here come from oox/source/vml/vmltextbox.cxx.
+ // It probably could safely expand to all Ids that shapes support.
+ const PropertyIds eIds[] = {
+ PROP_CHAR_HEIGHT,
+ PROP_CHAR_FONT_NAME,
+ PROP_CHAR_WEIGHT,
+ PROP_CHAR_CHAR_KERNING,
+ PROP_CHAR_COLOR,
+ PROP_PARA_ADJUST
+ };
+ const uno::Reference<beans::XPropertyState> xSyncedPropertyState(xSyncedPropertySet, uno::UNO_QUERY_THROW);
+ for ( const auto& eId : eIds )
+ {
+ try
+ {
+ if ( bOnlyApplyCharHeight && eId != PROP_CHAR_HEIGHT )
+ continue;
+
+ const OUString sPropName = getPropertyName(eId);
+ if ( beans::PropertyState_DEFAULT_VALUE == xSyncedPropertyState->getPropertyState(sPropName) )
+ {
+ const uno::Any aProp = GetPropertyFromStyleSheet(eId, pEntry, /*bDocDefaults=*/true, /*bPara=*/true);
+ if ( aProp.hasValue() )
+ xSyncedPropertySet->setPropertyValue( sPropName, aProp );
+ }
+ }
+ catch (uno::Exception&) {}
+ }
+ }
+ }
+ catch (uno::Exception&) {}
+ }
+
// A GroupShape doesn't implement text::XTextRange, but appending
// an empty reference to the stacks still makes sense, because this
// way bToRemove can be set, and we won't end up with duplicated
diff --git a/writerfilter/source/dmapper/StyleSheetTable.cxx b/writerfilter/source/dmapper/StyleSheetTable.cxx
index 08881a93a9fe..ded569a09418 100644
--- a/writerfilter/source/dmapper/StyleSheetTable.cxx
+++ b/writerfilter/source/dmapper/StyleSheetTable.cxx
@@ -779,20 +779,6 @@ void StyleSheetTable::lcl_sprm(Sprm & rSprm)
m_pImpl->m_pCurrentEntry->pProperties->InsertProps(pProps);
m_pImpl->m_rDMapper.PopStyleSheetProperties( );
-
- if (m_pImpl->m_pCurrentEntry->nStyleTypeCode == STYLE_TYPE_PARA && m_pImpl->m_pCurrentEntry->bIsDefaultStyle)
- {
- // The current style is the default paragraph style.
- PropertyMapPtr pProperties = m_pImpl->m_pCurrentEntry->pProperties;
- if (pProperties->isSet(PROP_CHAR_HEIGHT) && !m_pImpl->m_pDefaultParaProps->isSet(PROP_CHAR_HEIGHT))
- {
- // We provide a character height value, but a document-level default wasn't set.
- if (m_pImpl->m_xTextDefaults.is())
- {
- m_pImpl->m_xTextDefaults->setPropertyValue("CharHeight", pProperties->getProperty(PROP_CHAR_HEIGHT)->second);
- }
- }
- }
}
}
break;
More information about the Libreoffice-commits
mailing list