sw/qa/extras/ooxmlexport/data/tdf105035_framePrB.docx |binary
 sw/qa/extras/ooxmlexport/data/tdf105035_framePrC.docx |binary
 sw/qa/extras/ooxmlexport/ooxmlexport18.cxx            |   29 ++++
 writerfilter/source/dmapper/DomainMapper_Impl.cxx     |  106 +++++++++++++++---
 writerfilter/source/dmapper/DomainMapper_Impl.hxx     |    2 
 5 files changed, 121 insertions(+), 16 deletions(-)

New commits:
commit cbdd54e41a78e6a567d7ff97935721b07461cce5
Author:     Justin Luth <justin.l...@collabora.com>
AuthorDate: Wed Apr 5 08:19:11 2023 -0400
Commit:     Miklos Vajna <vmik...@collabora.com>
CommitDate: Thu Apr 13 08:19:09 2023 +0200

    tdf#105035 writerfilter framePr: don't allow overlap in extreme case
    
    I NEED to do something about preventing overlap
    because in certain cases MS Word (2010/2016 tested) does
    not overlap frames even though their vertical placement
    is identical.
    
    The previous commit just fixed mis-combining separate frames into one,
    but now those frames might overlap each other - which is worse
    than being combined into one IMHO.
    
    Unfortunately, Microsoft's decision to not overlap seems arbirary.
    In most cases it needs to be allowed,
    so I have developed a minimal set of specifications
    that prevent overlap for bug 105035's specific document,
    and it should be flexible enough to allow other cases to be added.
    I tested a number of cases when designing my formula.
    It doesn't seem to prevent overlap for page or margin position.
    It doesn't seem to prevent overlap when set to top/middle/bottom.
    It doesn't seem to prevent overlap if the frames are below the
    first line of text.
    
    I still have a problem: the order of the frames might
    layout wrong - as in this unit test. Sigh.
    
    Too bad this code is so ugly and long,
    but it should be relatively performant.
    
    make CppunitTest_sw_ooxmlexport18 CPPUNIT_TEST_NAME=testTdf105035_framePrB
    
    Change-Id: Iddaf36b9797a38b7906bb980e06cc73da7012eed
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/150063
    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/tdf105035_framePrB.docx 
b/sw/qa/extras/ooxmlexport/data/tdf105035_framePrB.docx
new file mode 100644
index 000000000000..fe813609fbac
Binary files /dev/null and 
b/sw/qa/extras/ooxmlexport/data/tdf105035_framePrB.docx differ
diff --git a/sw/qa/extras/ooxmlexport/data/tdf105035_framePrC.docx 
b/sw/qa/extras/ooxmlexport/data/tdf105035_framePrC.docx
new file mode 100644
index 000000000000..4954f8cc6294
Binary files /dev/null and 
b/sw/qa/extras/ooxmlexport/data/tdf105035_framePrC.docx differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx 
b/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx
index caf312ba9198..b485a8ea2e1c 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx
@@ -152,6 +152,35 @@ DECLARE_OOXMLEXPORT_TEST(testTdf127622_framePr, 
"tdf127622_framePr.docx")
     CPPUNIT_ASSERT_EQUAL(1, getShapes());
 }
 
+DECLARE_OOXMLEXPORT_TEST(testTdf105035_framePrB, "tdf105035_framePrB.docx")
+{
+    // The paragraphs have different frame definitions, so they must be in 
separate frames,
+    // and the frames must not overlap - even though their vertical positions 
are identical.
+    const auto& pLayout = parseLayoutDump();
+    sal_Int32 n1stFlyBottom
+        = getXPath(pLayout, "//page[1]//anchored/fly[1]/infos/bounds", 
"bottom").toInt32();
+    sal_Int32 n2ndFlyTop
+        = getXPath(pLayout, "//page[1]//anchored/fly[2]/infos/bounds", 
"top").toInt32();
+    CPPUNIT_ASSERT_GREATER(n1stFlyBottom, n2ndFlyTop); //Top is greater than 
bottom
+
+    // Impossible layout TODO: the textboxes are in the wrong order.
+    OUString sTextBox1("Preparation of Papers for IEEE TRANSACTIONS and 
JOURNALS (November 2012)");
+    CPPUNIT_ASSERT_MESSAGE("DID YOU FIX ME? Wow - I didn't think this would be 
possible!",
+        !getXPathContent(pLayout, 
"//page[1]//anchored/fly[1]/txt").startsWith(sTextBox1));
+}
+
+DECLARE_OOXMLEXPORT_TEST(testTdf105035_framePrC, "tdf105035_framePrC.docx")
+{
+    // The paragraphs have different frame definitions, so they must be in 
separate frames,
+    // and the frames DO overlap this time.
+    const auto& pLayout = parseLayoutDump();
+    sal_Int32 n1stFlyTop
+        = getXPath(pLayout, "//page[1]//anchored/fly[1]/infos/bounds", 
"top").toInt32();
+    sal_Int32 n2ndFlyTop
+        = getXPath(pLayout, "//page[1]//anchored/fly[2]/infos/bounds", 
"top").toInt32();
+    CPPUNIT_ASSERT_EQUAL(n1stFlyTop, n2ndFlyTop); //both frames start at the 
same position
+}
+
 DECLARE_OOXMLEXPORT_TEST(testTdf154129_framePr1, "tdf154129_framePr1.docx")
 {
     for (size_t i = 1; i < 4; ++i)
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx 
b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index 11247cbf6b77..6a7e9a405ee1 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -1803,7 +1803,7 @@ DomainMapper_Impl::MakeFrameProperties(const 
ParagraphProperties& rProps)
     return aFrameProperties;
 }
 
-void DomainMapper_Impl::CheckUnregisteredFrameConversion()
+void DomainMapper_Impl::CheckUnregisteredFrameConversion(bool bPreventOverlap)
 {
     if (m_aTextAppendStack.empty())
         return;
@@ -1827,6 +1827,9 @@ void DomainMapper_Impl::CheckUnregisteredFrameConversion()
             comphelper::makePropertyValue(getPropertyName(PROP_FRM_DIRECTION), 
*nDirection));
     }
 
+    if (bPreventOverlap)
+        
aFrameProperties.push_back(comphelper::makePropertyValue("AllowOverlap", 
uno::Any(false)));
+
     // If there is no fill, the Word default is 100% transparency.
     // Otherwise CellColorHandler has priority, and this setting
     // will be ignored.
@@ -2263,24 +2266,97 @@ void DomainMapper_Impl::finishParagraph( const 
PropertyMapPtr& pPropertyMap, con
                     if( pParaContext->props().IsFrameMode() )
                         pToBeSavedProperties = new 
ParagraphProperties(pParaContext->props());
                 }
-                else if (pParaContext->props().IsFrameMode()
-                         && 
MakeFrameProperties(*rAppendContext.pLastParagraphProperties)
-                             == MakeFrameProperties(pParaContext->props()))
-                {
-                    //handles (7)
-                    
rAppendContext.pLastParagraphProperties->SetEndingRange(rAppendContext.xInsertPosition.is()
 ? rAppendContext.xInsertPosition : xTextAppend->getEnd());
-                    bKeepLastParagraphProperties = true;
-                }
                 else
                 {
-                    //handles (8)(9) and completes (6)
-                    CheckUnregisteredFrameConversion( );
+                    const bool 
bIsFrameMode(pParaContext->props().IsFrameMode());
+                    std::vector<beans::PropertyValue> aCurrFrameProperties;
+                    std::vector<beans::PropertyValue> aPrevFrameProperties;
+                    if (bIsFrameMode)
+                    {
+                        aCurrFrameProperties = 
MakeFrameProperties(pParaContext->props());
+                        aPrevFrameProperties
+                            = 
MakeFrameProperties(*rAppendContext.pLastParagraphProperties);
+                    }
 
-                    // If different frame properties are set on this 
paragraph, keep them.
-                    if ( !bIsDropCap && pParaContext->props().IsFrameMode() )
+                    if (bIsFrameMode && aPrevFrameProperties == 
aCurrFrameProperties)
                     {
-                        pToBeSavedProperties = new 
ParagraphProperties(pParaContext->props());
-                        lcl_AddRange(pToBeSavedProperties, xTextAppend, 
rAppendContext);
+                        //handles (7)
+                        
rAppendContext.pLastParagraphProperties->SetEndingRange(
+                            rAppendContext.xInsertPosition.is() ? 
rAppendContext.xInsertPosition
+                                                                : 
xTextAppend->getEnd());
+                        bKeepLastParagraphProperties = true;
+                    }
+                    else
+                    {
+                        // handles (8)(9) and completes (6)
+
+                        // RTF has an \overlap flag (which we ignore so far)
+                        // but DOCX has nothing like that for framePr
+                        // Always allow overlap in the RTF case - so there can 
be no regression.
+
+                        // In MSO UI, there is no setting for AllowOverlap for 
this kind of frame.
+                        // Although they CAN overlap with other anchored 
things,
+                        // they do not _easily_ overlap with other framePr's,
+                        // so when one frame follows another (8), don't let 
the first be overlapped.
+                        bool bPreventOverlap = !IsRTFImport() && bIsFrameMode 
&& !bIsDropCap;
+
+                        // Preventing overlap is emulation - so deny overlap 
as little as possible.
+                        sal_Int16 nVertOrient = text::VertOrientation::NONE;
+                        sal_Int16 nVertOrientRelation = 
text::RelOrientation::FRAME;
+                        sal_Int32 nCurrVertPos = 0;
+                        sal_Int32 nPrevVertPos = 0;
+                        for (size_t i = 0; bPreventOverlap && i < 
aCurrFrameProperties.size(); ++i)
+                        {
+                            if (aCurrFrameProperties[i].Name == 
"VertOrientRelation")
+                            {
+                                aCurrFrameProperties[i].Value >>= 
nVertOrientRelation;
+                                if (nVertOrientRelation != 
text::RelOrientation::FRAME)
+                                    bPreventOverlap = false;
+                            }
+                            else if (aCurrFrameProperties[i].Name == 
"VertOrient")
+                            {
+                                aCurrFrameProperties[i].Value >>= nVertOrient;
+                                if (nVertOrient != text::VertOrientation::NONE)
+                                    bPreventOverlap = false;
+                            }
+                            else if (aCurrFrameProperties[i].Name == 
"VertOrientPosition")
+                            {
+                                aCurrFrameProperties[i].Value >>= nCurrVertPos;
+                                // arbitrary value. Assume it must be less 
than 1st line height
+                                if (nCurrVertPos > 20 || nCurrVertPos < -20)
+                                    bPreventOverlap = false;
+                            }
+                        }
+                        for (size_t i = 0; bPreventOverlap && i < 
aPrevFrameProperties.size(); ++i)
+                        {
+                            if (aPrevFrameProperties[i].Name == 
"VertOrientRelation")
+                            {
+                                aPrevFrameProperties[i].Value >>= 
nVertOrientRelation;
+                                if (nVertOrientRelation != 
text::RelOrientation::FRAME)
+                                    bPreventOverlap = false;
+                            }
+                            else if (aPrevFrameProperties[i].Name == 
"VertOrient")
+                            {
+                                aPrevFrameProperties[i].Value >>= nVertOrient;
+                                if (nVertOrient != text::VertOrientation::NONE)
+                                    bPreventOverlap = false;
+                            }
+                            else if (aPrevFrameProperties[i].Name == 
"VertOrientPosition")
+                            {
+                                aPrevFrameProperties[i].Value >>= nPrevVertPos;
+                                if (nPrevVertPos != nCurrVertPos)
+                                    bPreventOverlap = false;
+                            }
+                        }
+
+                        CheckUnregisteredFrameConversion(bPreventOverlap);
+
+                        // If different frame properties are set on this 
paragraph, keep them.
+                        if (!bIsDropCap && bIsFrameMode)
+                        {
+                            pToBeSavedProperties = new 
ParagraphProperties(pParaContext->props());
+                            lcl_AddRange(pToBeSavedProperties, xTextAppend, 
rAppendContext);
+                        }
                     }
                 }
             }
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.hxx 
b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
index 15cf63dec626..38e214ee5b2f 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.hxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
@@ -1043,7 +1043,7 @@ public:
     bool IsInComments() const { return m_bIsInComments; };
 
     std::vector<css::beans::PropertyValue> MakeFrameProperties(const 
ParagraphProperties& rProps);
-    void CheckUnregisteredFrameConversion( );
+    void CheckUnregisteredFrameConversion(bool bPreventOverlap = false);
 
     void RegisterFrameConversion(css::uno::Reference<css::text::XTextRange> 
const& xFrameStartRange,
                                  css::uno::Reference<css::text::XTextRange> 
const& xFrameEndRange,

Reply via email to