Resolves: tdf#163554 Read/write Excel 3D reference as 'Sheet1:Sheet2'!C4

... if at least one of the sheet names has to be quoted, instead
of 'Sheet1':'Sheet2'!C4:C4 where 'Sheet1':'Sheet2' is more logical
but not understood neither by Excel nor was it by Calc. A single
C4 instead of C4:C4 is more cosmetical.

Accept both forms now when reading OOXML documents to be able to
read back such broken expressions.

Change-Id: I228e458e2e3af387d9f89900fcfc16ab89757bae
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/175514
Tested-by: Jenkins
Reviewed-by: Eike Rathke <erack@redhat.com>
(cherry picked from commit 472f17d203207f1ef9d75ecfc9facda2d5f43384)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/175495
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>

3 files changed, 83 insertions, 10 deletions

diff --git a/sc/inc/compiler.hxx b/sc/inc/compiler.hxx
index 33fe7ca97361..04aa7ea18d2d 100644
--- a/sc/inc/compiler.hxx
+++ b/sc/inc/compiler.hxx
@@ -412,6 +412,23 @@ public:
     SC_DLLPUBLIC static void CheckTabQuotes( OUString& aTabName,
                                 const formula::FormulaGrammar::AddressConvention eConv = formula::FormulaGrammar::CONV_OOO );
 
+    /** Concatenates two sheet names in Excel syntax, i.e. 'Sheet1:Sheet2'
+        instead of 'Sheet1':'Sheet2' or 'Sheet1':Sheet2 or Sheet1:'Sheet2'.
+
+        @param  rBuf
+                Contains the first sheet name already, in the correct quoted 'Sheet''1' or
+                unquoted Sheet1 form if plain name.
+
+        @param  nQuotePos
+                Start position, of the first sheet name if unquoted, or its
+                opening quote.
+
+        @param  rEndTabName
+                Second sheet name to append, in the correct quoted 'Sheet''2'
+                or unquoted Sheet2 form if plain name.
+     */
+    static void FormExcelSheetRange( OUStringBuffer& rBuf, sal_Int32 nQuotePos, const OUString& rEndTabName );
+
     /** Analyzes a string for a 'Doc'#Tab construct, or 'Do''c'#Tab etc...
 
         @returns the position of the unquoted # hash mark in 'Doc'#Tab, or
diff --git a/sc/source/core/tool/address.cxx b/sc/source/core/tool/address.cxx
index 5ad3edee405f..773dc2343338 100644
--- a/sc/source/core/tool/address.cxx
+++ b/sc/source/core/tool/address.cxx
@@ -333,7 +333,7 @@ static const sal_Unicode * lcl_XL_ParseSheetRef( const sal_Unicode* start,
             aTabName += std::u16string_view( pCurrentStart, sal::static_int_cast<sal_Int32>( p - pCurrentStart));
         if (aTabName.isEmpty())
             return nullptr;
-        if (p == pMsoxlQuoteStop)
+        if (p == pMsoxlQuoteStop && *pMsoxlQuoteStop == '\'')
             ++p;    // position on ! of ...'!...
         if( *p != '!' && ( !bAllow3D || *p != ':' ) )
             return (!bAllow3D && *p == ':') ? p : start;
@@ -495,6 +495,7 @@ const sal_Unicode* ScRange::Parse_XL_Header(
     rEndTabName.clear();
     rExternDocName.clear();
     const sal_Unicode* pMsoxlQuoteStop = nullptr;
+    const sal_Unicode* pQuoted3DStop = nullptr;
     if (*p == '[')
     {
         ++p;
@@ -531,11 +532,18 @@ const sal_Unicode* ScRange::Parse_XL_Header(
         // 'E:\[EXTDATA8.XLS]Sheet1'!$A$7  or
         // 'E:\[EXTDATA12B.XLSB]Sheet1:Sheet3'!$A$11
         // But, 'Sheet1'!B3 would also be a valid!
-        // Excel does not allow [ and ] characters in sheet names though.
+        // Then again, 'Sheet1':'Sheet2'!C4 would be logical and Calc wrote
+        // that to OOXML but Excel instead does 'Sheet1:Sheet2'!C4 which is
+        // the worse you can get.
+        // Excel does not allow [ and ] and : characters in sheet names though.
         // But, more sickness comes with MOOXML as there may be
         // '[1]Sheet 4'!$A$1  where [1] is the external doc's index.
         p = parseQuotedName(p, rExternDocName);
-        if (*p != '!')
+        if (*p == ':')
+        {
+            // The incorrect 'Sheet1':'Sheet2' case. Just fall through.
+        }
+        else if (*p != '!')
         {
             rExternDocName.clear();
             return start;
@@ -544,7 +552,24 @@ const sal_Unicode* ScRange::Parse_XL_Header(
         {
             sal_Int32 nOpen = rExternDocName.indexOf( '[');
             if (nOpen == -1)
+            {
                 rExternDocName.clear();
+                // Look for 'Sheet1:Sheet2'!
+                if (*p == '!')
+                {
+                    const sal_Unicode* pQ = start + 1;
+                    do
+                    {
+                        if (*pQ == ':')
+                        {
+                            pMsoxlQuoteStop = pQ;
+                            pQuoted3DStop = p - 1;
+                            break;
+                        }
+                    }
+                    while (++pQ < p);
+                }
+            }
             else
             {
                 sal_Int32 nClose = rExternDocName.indexOf( ']', nOpen+1);
@@ -574,7 +599,7 @@ const sal_Unicode* ScRange::Parse_XL_Header(
             }
         }
         if (rExternDocName.isEmpty())
-            p = start;
+            p = (pQuoted3DStop ? start + 1 : start);
     }
 
     startTabs = p;
@@ -590,7 +615,8 @@ const sal_Unicode* ScRange::Parse_XL_Header(
         if( *p == ':' ) // 3d ref
         {
             startEndTabs = p + 1;
-            p = lcl_XL_ParseSheetRef( startEndTabs, rEndTabName, false, pMsoxlQuoteStop, pErrRef);
+            p = lcl_XL_ParseSheetRef( startEndTabs, rEndTabName, false,
+                    (pQuoted3DStop ? pQuoted3DStop : pMsoxlQuoteStop), pErrRef);
             if( p == nullptr )
             {
                 nFlags = nSaveFlags;
@@ -2142,6 +2168,7 @@ static void lcl_ScRange_Format_XL_Header( OUStringBuffer& rString, const ScRange
     if( !(nFlags & ScRefFlags::TAB_3D) )
         return;
 
+    sal_Int32 nQuotePos = rString.getLength();
     OUString aTabName, aDocName;
     lcl_Split_DocTab( rDoc, rRange.aStart.Tab(), rDetails, nFlags, aTabName, aDocName );
     switch (rDetails.eConv)
@@ -2164,6 +2191,7 @@ static void lcl_ScRange_Format_XL_Header( OUStringBuffer& rString, const ScRange
             if (!aDocName.isEmpty())
             {
                 rString.append("[" + aDocName + "]");
+                nQuotePos = rString.getLength();
             }
             rString.append(aTabName);
         break;
@@ -2171,8 +2199,7 @@ static void lcl_ScRange_Format_XL_Header( OUStringBuffer& rString, const ScRange
     if( nFlags & ScRefFlags::TAB2_3D )
     {
         lcl_Split_DocTab( rDoc, rRange.aEnd.Tab(), rDetails, nFlags, aTabName, aDocName );
-        rString.append(":");
-        rString.append(aTabName);
+        ScCompiler::FormExcelSheetRange( rString, nQuotePos, aTabName);
     }
     rString.append("!");
 }
diff --git a/sc/source/core/tool/compiler.cxx b/sc/source/core/tool/compiler.cxx
index 7169d269486d..26684473bc44 100644
--- a/sc/source/core/tool/compiler.cxx
+++ b/sc/source/core/tool/compiler.cxx
@@ -1179,11 +1179,11 @@ struct ConventionXL
             GetTab(rLimits, rPos, rTabNames, rRef.Ref2, aEndTabName);
         }
 
+        const sal_Int32 nQuotePos = rBuf.getLength();
         rBuf.append( aStartTabName );
         if( !bSingleRef && rRef.Ref2.IsFlag3D() && aStartTabName != aEndTabName )
         {
-            rBuf.append( ':' );
-            rBuf.append( aEndTabName );
+            ScCompiler::FormExcelSheetRange( rBuf, nQuotePos, aEndTabName);
         }
 
         rBuf.append( '!' );
@@ -1386,7 +1386,7 @@ struct ConventionXL_A1 : public Convention_A1, public ConventionXL
         }
 
         makeSingleCellStr(rLimits, rBuf, aRef.Ref1, aAbs1);
-        if (!bSingleRef)
+        if (!bSingleRef && (aAbs1.Row() != aAbs2.Row() || aAbs1.Col() != aAbs2.Col()))
         {
             rBuf.append( ':' );
             makeSingleCellStr(rLimits, rBuf, aRef.Ref2, aAbs2);
@@ -1729,6 +1729,12 @@ struct ConventionXL_R1C1 : public ScCompiler::Convention, public ConventionXL
 
         r1c1_add_row(rBuf, rRef.Ref1, aAbsRef.aStart);
         r1c1_add_col(rBuf, rRef.Ref1, aAbsRef.aStart);
+        // We can't parse a single col/row reference in the context of a R1C1
+        // 3D reference back yet, otherwise (if Excel understands it) an
+        // additional condition similar to ConventionXL_A1::makeRefStr() could
+        // be
+        //
+        //   && (aAbsRef.aStart.Row() != aAbsRef.aEnd.Row() || aAbsRef.aStart.Col() != aAbsRef.aEnd.Col())
         if (!bSingleRef)
         {
             rBuf.append( ':' );
@@ -1994,6 +2000,29 @@ void ScCompiler::CheckTabQuotes( OUString& rString,
     }
 }
 
+void ScCompiler::FormExcelSheetRange( OUStringBuffer& rBuf, sal_Int32 nQuotePos, const OUString& rEndTabName )
+{
+    OUString aEndTabName(rEndTabName);
+    if (nQuotePos < rBuf.getLength())
+    {
+        const bool bQuoted2 = (!aEndTabName.isEmpty() && aEndTabName[0] == '\'');
+        if (bQuoted2)
+            aEndTabName = aEndTabName.subView(1);   //  Sheet2'
+        if (rBuf[nQuotePos] == '\'')                // 'Sheet1'
+        {
+            const sal_Int32 nLast = rBuf.getLength() - 1;
+            if (rBuf[nLast] == '\'')
+                rBuf.remove(nLast, 1);              // 'Sheet1
+        }
+        else if (bQuoted2)                          //  Sheet1
+        {
+            rBuf.insert(nQuotePos, '\'');           // 'Sheet1
+        }
+    }
+    rBuf.append( ':' );
+    rBuf.append( aEndTabName );
+}
+
 sal_Int32 ScCompiler::GetDocTabPos( const OUString& rString )
 {
     if (rString[0] != '\'')