[Libreoffice-commits] core.git: Branch 'distro/collabora/cp-6.2' - sw/qa writerfilter/source

Miklos Vajna (via logerrit) logerrit at kemper.freedesktop.org
Tue Dec 17 13:32:54 UTC 2019


 sw/qa/extras/ooxmlexport/data/tdf115719b.docx     |binary
 sw/qa/extras/ooxmlexport/ooxmlexport11.cxx        |   12 ++++++++++++
 writerfilter/source/dmapper/DomainMapper_Impl.cxx |   14 ++++++++++++--
 writerfilter/source/dmapper/DomainMapper_Impl.hxx |   15 +++++++++++----
 writerfilter/source/dmapper/GraphicImport.cxx     |    7 +++++++
 writerfilter/source/dmapper/GraphicImport.hxx     |    1 +
 writerfilter/source/dmapper/PropertyMap.cxx       |   13 +++++++------
 7 files changed, 50 insertions(+), 12 deletions(-)

New commits:
commit 74b026011dc66e9b60579f80dba6ee2385c81fb9
Author:     Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Fri Dec 6 16:54:07 2019 +0100
Commit:     Miklos Vajna <vmiklos at collabora.com>
CommitDate: Tue Dec 17 14:32:04 2019 +0100

    Related: tdf#115719 DOCX import: fix increased spacing vs left-aligned objects
    
    Commit 8b73bafbc18acb4dd8911d2f2de8158d98eb6144 (tdf#115719 DOCX import:
    increase paragraph spacing for anchored objects, 2018-02-14) added an
    import-time tweak for a problem that has been confirmed to be a Word
    layout bug in the meantime (and the tweak makes Writer behave the same
    way if the document has been created by an affected Word version for
    layout compatiblity).
    
    Later, commit 4883da6fd25e4645a3b30cb58212a2f666dae75a (Related:
    tdf#124600 DOCX import: ignore left wrap on left-aligned shapes,
    2018-02-14) fixed left spacing of anchored objects aligned to the left,
    to be in sync with what the DOC import does.
    
    This broke the previous fix in case the shapes are left-aligned.
    
    Fix the problem by tracking what is the in-file-format and logical left
    margin, so the final doc model has the value necessary for correct
    horizontal positioning and the importer has the value that's necessary
    for correct vertical positioning.
    
    (cherry picked from commit 814cb2433da6bd608e935fa5531d2a2b92867985)
    
    Conflicts:
            writerfilter/source/dmapper/PropertyMap.cxx
    
    Change-Id: I8f16cbe7bad40e243111c902bdc1ab0e8141d6b9
    Reviewed-on: https://gerrit.libreoffice.org/85265
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
    Reviewed-by: Miklos Vajna <vmiklos at collabora.com>

diff --git a/sw/qa/extras/ooxmlexport/data/tdf115719b.docx b/sw/qa/extras/ooxmlexport/data/tdf115719b.docx
new file mode 100644
index 000000000000..a632e3df0e7a
Binary files /dev/null and b/sw/qa/extras/ooxmlexport/data/tdf115719b.docx differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport11.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport11.cxx
index 87019efcec3d..7af3a9ff3b06 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport11.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport11.cxx
@@ -309,6 +309,18 @@ DECLARE_OOXMLEXPORT_TEST(testTdf115719, "tdf115719.docx")
     CPPUNIT_ASSERT_EQUAL(2, getPages());
 }
 
+DECLARE_OOXMLIMPORT_TEST(testTdf115719b, "tdf115719b.docx")
+{
+    // This is similar to testTdf115719, but here the left textbox is not aligned "from left, by
+    // 0cm" but simply aligned to left, which is a different codepath.
+
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected: 2
+    // - Actual  : 1
+    // i.e. the the textboxes did not appear on the 2nd page, but everything was on a single page.
+    CPPUNIT_ASSERT_EQUAL(2, getPages());
+}
+
 DECLARE_OOXMLEXPORT_TEST(testTdf123243, "tdf123243.docx")
 {
     // Without the accompanying fix in place, this test would have failed with 'Expected: 1; Actual:
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index a2b5f5fce1df..037233cb769b 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -1469,7 +1469,7 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con
                         // Remember what objects are anchored to this paragraph.
                         // That list is only used for Word compat purposes, and
                         // it is only relevant for body text.
-                        AnchoredObjectInfo aInfo;
+                        AnchoredObjectsInfo aInfo;
                         aInfo.m_xParagraph = xTextRange;
                         aInfo.m_aAnchoredObjects = rAppendContext.m_aAnchoredObjects;
                         m_aAnchoredObjectAnchors.push_back(aInfo);
@@ -5604,8 +5604,18 @@ void  DomainMapper_Impl::ImportGraphic(const writerfilter::Reference< Properties
         appendTextContent( xTextContent, uno::Sequence< beans::PropertyValue >() );
 
         if (eGraphicImportType == IMPORT_AS_DETECTED_ANCHOR && !m_aTextAppendStack.empty())
+        {
             // Remember this object is anchored to the current paragraph.
-            m_aTextAppendStack.top().m_aAnchoredObjects.push_back(xTextContent);
+            AnchoredObjectInfo aInfo;
+            aInfo.m_xAnchoredObject = xTextContent;
+            if (m_pGraphicImport)
+            {
+                // We still have the graphic import around, remember the original margin, so later
+                // SectionPropertyMap::HandleIncreasedAnchoredObjectSpacing() can use it.
+                aInfo.m_nLeftMargin = m_pGraphicImport->GetLeftMarginOrig();
+            }
+            m_aTextAppendStack.top().m_aAnchoredObjects.push_back(aInfo);
+        }
     }
 
     // Clear the reference, so in case the embedded object is inside a
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.hxx b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
index 9c342e4492a4..437d1fb73440 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.hxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
@@ -219,7 +219,7 @@ struct TextAppendContext
      * Objects anchored to the current paragraph, may affect the paragraph
      * spacing.
      */
-    std::vector<css::uno::Reference<css::text::XTextContent>> m_aAnchoredObjects;
+    std::vector<AnchoredObjectInfo> m_aAnchoredObjects;
 
     TextAppendContext(const css::uno::Reference<css::text::XTextAppend>& xAppend, const css::uno::Reference<css::text::XTextCursor>& xCur)
         : xTextAppend(xAppend)
@@ -380,11 +380,18 @@ struct FloatingTableInfo
     css::uno::Any getPropertyValue(const OUString &propertyName);
 };
 
-/// Stores info about objects anchored to a given paragraph.
+/// Stores original/in-file-format info about a single anchored object.
 struct AnchoredObjectInfo
+{
+    css::uno::Reference<css::text::XTextContent> m_xAnchoredObject;
+    sal_Int32 m_nLeftMargin = 0;
+};
+
+/// Stores info about objects anchored to a given paragraph.
+struct AnchoredObjectsInfo
 {
     css::uno::Reference<css::text::XTextRange> m_xParagraph;
-    std::vector<css::uno::Reference<css::text::XTextContent>> m_aAnchoredObjects;
+    std::vector<AnchoredObjectInfo> m_aAnchoredObjects;
 };
 
 struct SymbolData
@@ -958,7 +965,7 @@ public:
     std::vector<FloatingTableInfo> m_aPendingFloatingTables;
 
     /// Paragraphs with anchored objects in the current section.
-    std::vector<AnchoredObjectInfo> m_aAnchoredObjectAnchors;
+    std::vector<AnchoredObjectsInfo> m_aAnchoredObjectAnchors;
 
     /// Append a property to a sub-grabbag if necessary (e.g. 'lineRule', 'auto')
     void appendGrabBag(std::vector<css::beans::PropertyValue>& rInteropGrabBag, const OUString& aKey, const OUString& aValue);
diff --git a/writerfilter/source/dmapper/GraphicImport.cxx b/writerfilter/source/dmapper/GraphicImport.cxx
index 97cc3c78fe92..8afaad814777 100644
--- a/writerfilter/source/dmapper/GraphicImport.cxx
+++ b/writerfilter/source/dmapper/GraphicImport.cxx
@@ -202,6 +202,7 @@ public:
     WrapPolygon::Pointer_t mpWrapPolygon;
 
     sal_Int32 nLeftMargin;
+    sal_Int32 nLeftMarginOrig = 0;
     sal_Int32 nRightMargin;
     sal_Int32 nTopMargin;
     sal_Int32 nBottomMargin;
@@ -971,6 +972,7 @@ void GraphicImport::ProcessShapeOptions(Value const & rValue)
     {
         case NS_ooxml::LN_CT_Anchor_distL:
             m_pImpl->nLeftMargin = nIntValue / 360;
+            m_pImpl->nLeftMarginOrig = m_pImpl->nLeftMargin;
         break;
         case NS_ooxml::LN_CT_Anchor_distT:
             //todo: changes have to be applied depending on the orientation, see SwWW8ImplReader::AdjustULWrapForWordMargins()
@@ -1486,6 +1488,11 @@ bool GraphicImport::IsGraphic() const
     return m_pImpl->bIsGraphic;
 }
 
+sal_Int32 GraphicImport::GetLeftMarginOrig() const
+{
+    return m_pImpl->nLeftMarginOrig;
+}
+
 }
 }
 
diff --git a/writerfilter/source/dmapper/GraphicImport.hxx b/writerfilter/source/dmapper/GraphicImport.hxx
index ee3b5b7cdb05..70b02d353e8f 100644
--- a/writerfilter/source/dmapper/GraphicImport.hxx
+++ b/writerfilter/source/dmapper/GraphicImport.hxx
@@ -97,6 +97,7 @@ public:
     css::uno::Reference<css::text::XTextContent> GetGraphicObject();
     const css::uno::Reference<css::drawing::XShape>& GetXShapeObject() { return m_xShape;}
     bool IsGraphic() const;
+    sal_Int32 GetLeftMarginOrig() const;
 
  private:
     // Properties
diff --git a/writerfilter/source/dmapper/PropertyMap.cxx b/writerfilter/source/dmapper/PropertyMap.cxx
index f05e5a8d7d42..80351c6d1c66 100644
--- a/writerfilter/source/dmapper/PropertyMap.cxx
+++ b/writerfilter/source/dmapper/PropertyMap.cxx
@@ -1239,7 +1239,7 @@ void SectionPropertyMap::HandleIncreasedAnchoredObjectSpacing(DomainMapper_Impl&
     sal_Int32 nPageWidth = GetPageWidth();
     sal_Int32 nTextAreaWidth = nPageWidth - GetLeftMargin() - GetRightMargin();
 
-    std::vector<AnchoredObjectInfo>& rAnchoredObjectAnchors = rDM_Impl.m_aAnchoredObjectAnchors;
+    std::vector<AnchoredObjectsInfo>& rAnchoredObjectAnchors = rDM_Impl.m_aAnchoredObjectAnchors;
     for (auto& rAnchor : rAnchoredObjectAnchors)
     {
         // Ignore this paragraph when there are not enough shapes to trigger the Word bug we
@@ -1252,11 +1252,11 @@ void SectionPropertyMap::HandleIncreasedAnchoredObjectSpacing(DomainMapper_Impl&
         sal_Int32 nShapesWidth = 0;
         for (const auto& rAnchored : rAnchor.m_aAnchoredObjects)
         {
-            uno::Reference<drawing::XShape> xShape(rAnchored, uno::UNO_QUERY);
+            uno::Reference<drawing::XShape> xShape(rAnchored.m_xAnchoredObject, uno::UNO_QUERY);
             if (!xShape.is())
                 continue;
 
-            uno::Reference<beans::XPropertySet> xPropertySet(rAnchored, uno::UNO_QUERY);
+            uno::Reference<beans::XPropertySet> xPropertySet(xShape, uno::UNO_QUERY);
             if (!xPropertySet.is())
                 continue;
 
@@ -1266,8 +1266,9 @@ void SectionPropertyMap::HandleIncreasedAnchoredObjectSpacing(DomainMapper_Impl&
             if (eWrap == text::WrapTextMode_THROUGH)
                 continue;
 
-            sal_Int32 nLeftMargin = 0;
-            xPropertySet->getPropertyValue("LeftMargin") >>= nLeftMargin;
+            // Use the original left margin, in case GraphicImport::lcl_sprm() reduced the doc model
+            // one to 0.
+            sal_Int32 nLeftMargin = rAnchored.m_nLeftMargin;
             sal_Int32 nRightMargin = 0;
             xPropertySet->getPropertyValue("RightMargin") >>= nRightMargin;
             nShapesWidth += xShape->getSize().Width + nLeftMargin + nRightMargin;
@@ -1280,7 +1281,7 @@ void SectionPropertyMap::HandleIncreasedAnchoredObjectSpacing(DomainMapper_Impl&
         sal_Int32 nHeight = 0;
         for (const auto& rAnchored : rAnchor.m_aAnchoredObjects)
         {
-            uno::Reference<drawing::XShape> xShape(rAnchored, uno::UNO_QUERY);
+            uno::Reference<drawing::XShape> xShape(rAnchored.m_xAnchoredObject, uno::UNO_QUERY);
             if (!xShape.is())
                 continue;
 


More information about the Libreoffice-commits mailing list