sw: fix unwanted long vertical border around vertically merged Word cell

In case cells A1 and A2 are vertically merged, they can still specify
different border properties for the two cells. Word draws the border
properties of the covered cells, while Writer ignores the formatting of
A2 in this case and only uses the properties of the covering cell.

Table cell border collapsing rules already differ in Word and Writer, so
SwTabFramePainter::Insert() knows if the table cell's border is
Word-style or not. Extend this code to handle vertically merged cells
better.

In general, this means a cell no longer has a fixed set of 4 borders,
but each edge may have several sub-borders. This commit is a step in
that direction, and handles the case when a vertical (left or right)
border style is set initially, but not set at the end -- as we iterate
through the list of cells in a vertical merge.

Change-Id: I35a05097ce70243099307ce8066766aef61a2bc5
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/111954
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins

5 files changed, 228 insertions, 0 deletions

diff --git a/sw/qa/core/layout/data/vmerge-cell-border.docx b/sw/qa/core/layout/data/vmerge-cell-border.docx
new file mode 100644
index 000000000000..11d7a76d22ec
--- /dev/null
+++ b/sw/qa/core/layout/data/vmerge-cell-border.docx
diff --git a/sw/qa/core/layout/layout.cxx b/sw/qa/core/layout/layout.cxx
index 59af4962263f..7c899aa31d63 100644
--- a/sw/qa/core/layout/layout.cxx
+++ b/sw/qa/core/layout/layout.cxx
@@ -389,6 +389,80 @@ CPPUNIT_TEST_FIXTURE(SwCoreLayoutTest, testGutterMarginPageBorder)
 #endif
 }
 
+CPPUNIT_TEST_FIXTURE(SwCoreLayoutTest, testVerticallyMergedCellBorder)
+{
+    // Given a document with a table: 2 columns, 5 rows. B2 -> B5 is merged:
+    SwDoc* pDoc = createSwDoc(DATA_DIRECTORY, "vmerge-cell-border.docx");
+    SwDocShell* pShell = pDoc->GetDocShell();
+
+    // When rendering the table:
+    std::shared_ptr<GDIMetaFile> xMetaFile = pShell->GetPreviewMetaFile();
+
+    // Make sure that B4->B5 has no borders.
+    MetafileXmlDump dumper;
+    xmlDocUniquePtr pXmlDoc = dumpAndParse(dumper, *xMetaFile);
+    // Collect vertical positions of all border points.
+    xmlXPathObjectPtr pXmlObj = getXPathNode(pXmlDoc, "//polyline[@style='solid']/point");
+    xmlNodeSetPtr pXmlNodes = pXmlObj->nodesetval;
+    std::vector<sal_Int32> aBorderPositions;
+    for (int i = 0; i < xmlXPathNodeSetGetLength(pXmlNodes); ++i)
+    {
+        xmlNodePtr pXmlNode = pXmlNodes->nodeTab[i];
+        xmlChar* pValue = xmlGetProp(pXmlNode, BAD_CAST("y"));
+        sal_Int32 nValue = OString(reinterpret_cast<char const*>(pValue)).toInt32();
+        aBorderPositions.push_back(nValue);
+    }
+    xmlXPathFreeObject(pXmlObj);
+    // Collect top and bottom of the B1->B3 rows.
+    xmlDocUniquePtr pLayout = parseLayoutDump();
+    pXmlObj = getXPathNode(pLayout, "//tab/row/infos/bounds");
+    pXmlNodes = pXmlObj->nodesetval;
+    std::vector<sal_Int32> aLayoutPositions;
+    for (int i = 0; i < 3; ++i)
+    {
+        xmlNodePtr pXmlNode = pXmlNodes->nodeTab[i];
+        if (i == 0)
+        {
+            xmlChar* pValue = xmlGetProp(pXmlNode, BAD_CAST("top"));
+            sal_Int32 nValue = OString(reinterpret_cast<char const*>(pValue)).toInt32();
+            aLayoutPositions.push_back(nValue);
+        }
+        xmlChar* pValue = xmlGetProp(pXmlNode, BAD_CAST("bottom"));
+        sal_Int32 nValue = OString(reinterpret_cast<char const*>(pValue)).toInt32();
+        aLayoutPositions.push_back(nValue);
+    }
+    xmlXPathFreeObject(pXmlObj);
+    // Check if any border is outside the B1->B3 range.
+    for (const auto nBorderPosition : aBorderPositions)
+    {
+        bool bFound = false;
+        for (const auto nLayoutPosition : aLayoutPositions)
+        {
+            if (std::abs(nBorderPosition - nLayoutPosition) <= 15)
+            {
+                bFound = true;
+                break;
+            }
+        }
+        std::stringstream ss;
+        ss << "Bad vertical position for border point: " << nBorderPosition;
+        ss << " Expected positions: ";
+        for (size_t i = 0; i < aLayoutPositions.size(); ++i)
+        {
+            if (i > 0)
+            {
+                ss << ", ";
+            }
+            ss << aLayoutPositions[i];
+        }
+
+        // Without the accompanying fix in place, this test would have failed with:
+        // - Bad vertical position for border point: 5624 Expected positions: 3022, 3540, 4059, 4578
+        // i.e. the middle vertical border end was the bottom of B5, not bottom of B3.
+        CPPUNIT_ASSERT_MESSAGE(ss.str(), bFound);
+    }
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/inc/cellfrm.hxx b/sw/source/core/inc/cellfrm.hxx
index d2f8730691ed..e8f894af3058 100644
--- a/sw/source/core/inc/cellfrm.hxx
+++ b/sw/source/core/inc/cellfrm.hxx
@@ -62,6 +62,12 @@ public:
     const SwCellFrame& FindStartEndOfRowSpanCell( bool bStart ) const;
     tools::Long GetLayoutRowSpan() const;
 
+    /// If this is a vertically merged cell, then looks up its covered cell in rRow.
+    const SwCellFrame* GetCoveredCellInRow(const SwRowFrame& rRow) const;
+
+    /// If this is a vertically merged cell, then looks up its covered cells.
+    std::vector<const SwCellFrame*> GetCoveredCells() const;
+
     void dumpAsXmlAttributes(xmlTextWriterPtr writer) const override;
 };
 
diff --git a/sw/source/core/layout/paintfrm.cxx b/sw/source/core/layout/paintfrm.cxx
index c549b5ffeb46..e90ab0bad378 100644
--- a/sw/source/core/layout/paintfrm.cxx
+++ b/sw/source/core/layout/paintfrm.cxx
@@ -2241,11 +2241,14 @@ struct SwLineEntry
     SwTwips mnKey;
     SwTwips mnStartPos;
     SwTwips mnEndPos;
+    SwTwips mnLimitedEndPos;
 
     svx::frame::Style maAttribute;
 
     enum OverlapType { NO_OVERLAP, OVERLAP1, OVERLAP2, OVERLAP3 };
 
+    enum class VerticalType { LEFT, RIGHT };
+
 public:
     SwLineEntry( SwTwips nKey,
                  SwTwips nStartPos,
@@ -2253,6 +2256,12 @@ public:
                  const svx::frame::Style& rAttribute );
 
     OverlapType Overlaps( const SwLineEntry& rComp ) const;
+
+    /**
+     * Assuming that this entry is for a Word-style covering cell and the border matching eType is
+     * set, limit the end position of this border in case covered cells have no borders set.
+     */
+    void LimitVerticalEndPos(const SwFrame& rFrame, VerticalType eType);
 };
 
 }
@@ -2264,6 +2273,7 @@ SwLineEntry::SwLineEntry( SwTwips nKey,
     :   mnKey( nKey ),
         mnStartPos( nStartPos ),
         mnEndPos( nEndPos ),
+        mnLimitedEndPos(0),
         maAttribute( rAttribute )
 {
 }
@@ -2317,6 +2327,37 @@ SwLineEntry::OverlapType SwLineEntry::Overlaps( const SwLineEntry& rNew )  const
     return eRet;
 }
 
+void SwLineEntry::LimitVerticalEndPos(const SwFrame& rFrame, VerticalType eType)
+{
+    if (!rFrame.IsCellFrame())
+    {
+        return;
+    }
+
+    const auto& rCellFrame = static_cast<const SwCellFrame&>(rFrame);
+    std::vector<const SwCellFrame*> aCoveredCells = rCellFrame.GetCoveredCells();
+    // Iterate in reverse order, so we can stop at the first cell that has a border. This can
+    // determine what is the minimal end position that is safe to use as a limit.
+    for (auto it = aCoveredCells.rbegin(); it != aCoveredCells.rend(); ++it)
+    {
+        const SwCellFrame* pCoveredCell = *it;
+        SwBorderAttrAccess aAccess( SwFrame::GetCache(), pCoveredCell );
+        const SwBorderAttrs& rAttrs = *aAccess.Get();
+        const SvxBoxItem& rBox = rAttrs.GetBox();
+        if (eType == VerticalType::LEFT && rBox.GetLeft())
+        {
+            break;
+        }
+
+        if (eType == VerticalType::RIGHT && rBox.GetRight())
+        {
+            break;
+        }
+
+        mnLimitedEndPos = pCoveredCell->getFrameArea().Top();
+    }
+}
+
 namespace {
 
 struct lt_SwLineEntry
@@ -2460,6 +2501,12 @@ void SwTabFramePainter::PaintLines(OutputDevice& rDev, const SwRect& rRect) cons
             svx::frame::Style aStyles[ 7 ];
             aStyles[ 0 ] = rEntryStyle;
             FindStylesForLine( aStart, aEnd, aStyles, bHori );
+
+            if (!bHori && rEntry.mnLimitedEndPos)
+            {
+                aEnd.setY(rEntry.mnLimitedEndPos);
+            }
+
             SwRect aRepaintRect( aStart, aEnd );
 
             // the repaint rectangle has to be moved a bit for the centered lines:
@@ -2780,8 +2827,17 @@ void SwTabFramePainter::Insert(const SwFrame& rFrame, const SvxBoxItem& rBoxItem
 
     SwLineEntry aLeft  (nLeft,   nTop,  nBottom,
             bVert ? aB                         : (bR2L ? aR : aL));
+    if (bWordTableCell && rBoxItem.GetLeft())
+    {
+        aLeft.LimitVerticalEndPos(rFrame, SwLineEntry::VerticalType::LEFT);
+    }
+
     SwLineEntry aRight (nRight,  nTop,  nBottom,
             bVert ? (bBottomAsTop ? aB : aT) : (bR2L ? aL : aR));
+    if (bWordTableCell && rBoxItem.GetRight())
+    {
+        aRight.LimitVerticalEndPos(rFrame, SwLineEntry::VerticalType::RIGHT);
+    }
     SwLineEntry aTop   (nTop,    nLeft, nRight,
             bVert ? aL                         : (bBottomAsTop ? aB : aT));
     SwLineEntry aBottom(nBottom, nLeft, nRight,
@@ -2812,6 +2868,14 @@ void SwTabFramePainter::Insert( SwLineEntry& rNew, bool bHori )
     while ( aIter != pLineSet->end() && rNew.mnStartPos < rNew.mnEndPos )
     {
         const SwLineEntry& rOld = *aIter;
+
+        if (rOld.mnLimitedEndPos)
+        {
+            // Don't merge with this line entry as it ends sooner than mnEndPos.
+            ++aIter;
+            continue;
+        }
+
         const SwLineEntry::OverlapType nOverlapType = rOld.Overlaps( rNew );
 
         const svx::frame::Style& rOldAttr = rOld.maAttribute;
diff --git a/sw/source/core/layout/tabfrm.cxx b/sw/source/core/layout/tabfrm.cxx
index e6c8c2bc1a66..25b69f680b58 100644
--- a/sw/source/core/layout/tabfrm.cxx
+++ b/sw/source/core/layout/tabfrm.cxx
@@ -5497,6 +5497,90 @@ tools::Long SwCellFrame::GetLayoutRowSpan() const
     return  nRet;
 }
 
+const SwCellFrame* SwCellFrame::GetCoveredCellInRow(const SwRowFrame& rRow) const
+{
+    if (GetLayoutRowSpan() <= 1)
+    {
+        // Not merged vertically.
+        return nullptr;
+    }
+
+    for (const SwFrame* pCell = rRow.GetLower(); pCell; pCell = pCell->GetNext())
+    {
+        if (!pCell->IsCellFrame())
+        {
+            continue;
+        }
+
+        auto pCellFrame = static_cast<const SwCellFrame*>(pCell);
+        if (!pCellFrame->IsCoveredCell())
+        {
+            continue;
+        }
+
+        if (pCellFrame->getFrameArea().Left() != getFrameArea().Left())
+        {
+            continue;
+        }
+
+        if (pCellFrame->getFrameArea().Width() != getFrameArea().Width())
+        {
+            continue;
+        }
+
+        // pCellFrame is covered, there are only covered cell frames between "this" and pCellFrame
+        // and the horizontal position/size matches "this".
+        return pCellFrame;
+    }
+
+    return nullptr;
+}
+
+std::vector<const SwCellFrame*> SwCellFrame::GetCoveredCells() const
+{
+    std::vector<const SwCellFrame*> aRet;
+    if (GetLayoutRowSpan() <= 1)
+    {
+        return aRet;
+    }
+
+    if (!GetUpper()->IsRowFrame())
+    {
+        return aRet;
+    }
+
+    auto pFirstRowFrame = static_cast<const SwRowFrame*>(GetUpper());
+    if (!pFirstRowFrame->GetNext())
+    {
+        return aRet;
+    }
+
+    if (!pFirstRowFrame->GetNext()->IsRowFrame())
+    {
+        return aRet;
+    }
+
+    for (const SwFrame* pRow = pFirstRowFrame->GetNext(); pRow; pRow = pRow->GetNext())
+    {
+        if (!pRow->IsRowFrame())
+        {
+            continue;
+        }
+
+        auto pRowFrame = static_cast<const SwRowFrame*>(pRow);
+        const SwCellFrame* pCovered = GetCoveredCellInRow(*pRowFrame);
+        if (!pCovered)
+        {
+            continue;
+        }
+
+        // Found a cell in a next row that is covered by "this".
+        aRet.push_back(pCovered);
+    }
+
+    return aRet;
+}
+
 void SwCellFrame::dumpAsXmlAttributes(xmlTextWriterPtr pWriter) const
 {
     SwFrame::dumpAsXmlAttributes(pWriter);