Fixed ODS import filter to correctly identify data layout dimension.

And cleaned up the code a bit.  The new code should be much more
robust.

4 files changed, 96 insertions, 61 deletions

diff --git a/sc/inc/dpoutputgeometry.hxx b/sc/inc/dpoutputgeometry.hxx
index e0a5a8706b44..f031ec1365e3 100644
--- a/sc/inc/dpoutputgeometry.hxx
+++ b/sc/inc/dpoutputgeometry.hxx
@@ -38,9 +38,8 @@ class SC_DLLPUBLIC ScDPOutputGeometry
 {
 public:
     enum FieldType { Column, Row, Page, Data, None };
-    enum ImportType { ODF, XLS };
 
-    ScDPOutputGeometry(const ScRange& rOutRange, bool bShowFilter, ImportType eImportType);
+    ScDPOutputGeometry(const ScRange& rOutRange, bool bShowFilter);
     ~ScDPOutputGeometry();
 
     /**
@@ -57,7 +56,7 @@ public:
 
     SCROW getRowFieldHeaderRow() const;
 
-    FieldType getFieldButtonType(const ScAddress& rPos) const;
+    std::pair<FieldType, size_t> getFieldButtonType(const ScAddress& rPos) const;
 
 private:
     ScDPOutputGeometry(); // disabled
@@ -69,8 +68,6 @@ private:
     sal_uInt32  mnPageFields;
     sal_uInt32  mnDataFields;
 
-    ImportType  meImportType;
-
     bool        mbShowFilter;
 };
 
diff --git a/sc/source/core/data/dpoutputgeometry.cxx b/sc/source/core/data/dpoutputgeometry.cxx
index 2defc5a6f22c..83a998f73ee4 100644
--- a/sc/source/core/data/dpoutputgeometry.cxx
+++ b/sc/source/core/data/dpoutputgeometry.cxx
@@ -38,13 +38,12 @@
 
 using ::std::vector;
 
-ScDPOutputGeometry::ScDPOutputGeometry(const ScRange& rOutRange, bool bShowFilter, ImportType eImportType) :
+ScDPOutputGeometry::ScDPOutputGeometry(const ScRange& rOutRange, bool bShowFilter) :
     maOutRange(rOutRange),
     mnRowFields(0),
     mnColumnFields(0),
     mnPageFields(0),
     mnDataFields(0),
-    meImportType(eImportType),
     mbShowFilter(bShowFilter)
 {
 }
@@ -163,13 +162,11 @@ SCROW ScDPOutputGeometry::getRowFieldHeaderRow() const
     return nCurRow;
 }
 
-ScDPOutputGeometry::FieldType ScDPOutputGeometry::getFieldButtonType(const ScAddress& rPos) const
+std::pair<ScDPOutputGeometry::FieldType, size_t>
+ScDPOutputGeometry::getFieldButtonType(const ScAddress& rPos) const
 {
     // We will ignore the table position for now.
 
-    bool bExtraTitleRow = (mnColumnFields == 0 && meImportType == ScDPOutputGeometry::XLS);
-    bool bDataLayout = mnDataFields > 1;
-
     SCROW nCurRow = maOutRange.aStart.Row();
 
     if (mnPageFields)
@@ -178,7 +175,10 @@ ScDPOutputGeometry::FieldType ScDPOutputGeometry::getFieldButtonType(const ScAdd
         SCROW nRowStart = maOutRange.aStart.Row() + mbShowFilter;
         SCROW nRowEnd   = nRowStart + static_cast<SCCOL>(mnPageFields-1);
         if (rPos.Col() == nCol && nRowStart <= rPos.Row() && rPos.Row() <= nRowEnd)
-            return Page;
+        {
+            size_t nPos = static_cast<size_t>(rPos.Row() - nRowStart);
+            return std::pair<FieldType, size_t>(Page, nPos);
+        }
 
         nCurRow = nRowEnd + 2;
     }
@@ -188,15 +188,17 @@ ScDPOutputGeometry::FieldType ScDPOutputGeometry::getFieldButtonType(const ScAdd
     if (mnColumnFields)
     {
         SCROW nRow = nCurRow;
-        SCCOL nColStart = static_cast<SCCOL>(maOutRange.aStart.Col() + mnRowFields + (bDataLayout ? 1 : 0));
+        SCCOL nColStart = static_cast<SCCOL>(maOutRange.aStart.Col() + mnRowFields);
         SCCOL nColEnd = nColStart + static_cast<SCCOL>(mnColumnFields-1);
         if (rPos.Row() == nRow && nColStart <= rPos.Col() && rPos.Col() <= nColEnd)
-            return Column;
+        {
+            size_t nPos = static_cast<size_t>(rPos.Col() - nColStart);
+            return std::pair<FieldType, size_t>(Column, nPos);
+        }
 
         nCurRow += static_cast<SCROW>(mnColumnFields);
     }
-
-    if (bExtraTitleRow)
+    else
         ++nCurRow;
 
     if (mnRowFields)
@@ -204,10 +206,13 @@ ScDPOutputGeometry::FieldType ScDPOutputGeometry::getFieldButtonType(const ScAdd
         SCCOL nColStart = maOutRange.aStart.Col();
         SCCOL nColEnd = nColStart + static_cast<SCCOL>(mnRowFields-1);
         if (rPos.Row() == nCurRow && nColStart <= rPos.Col() && rPos.Col() <= nColEnd)
-            return Row;
+        {
+            size_t nPos = static_cast<size_t>(rPos.Col() - nColStart);
+            return std::pair<FieldType, size_t>(Row, nPos);
+        }
     }
 
-    return None;
+    return std::pair<FieldType, size_t>(None, 0);
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/source/filter/excel/xipivot.cxx b/sc/source/filter/excel/xipivot.cxx
index 6cd0f48f6296..89465817d665 100644
--- a/sc/source/filter/excel/xipivot.cxx
+++ b/sc/source/filter/excel/xipivot.cxx
@@ -1470,7 +1470,7 @@ void XclImpPivotTable::ApplyMergeFlags(const ScRange& rOutRange, const ScDPSaveD
 {
     // Apply merge flags for varoius datapilot controls.
 
-    ScDPOutputGeometry aGeometry(rOutRange, false, ScDPOutputGeometry::XLS);
+    ScDPOutputGeometry aGeometry(rOutRange, false);
     aGeometry.setColumnFieldCount(maPTInfo.mnColFields);
     aGeometry.setPageFieldCount(maPTInfo.mnPageFields);
     aGeometry.setDataFieldCount(maPTInfo.mnDataFields);
diff --git a/sc/source/filter/xml/xmldpimp.cxx b/sc/source/filter/xml/xmldpimp.cxx
index 4d48946d3b4c..91d19cf37169 100644
--- a/sc/source/filter/xml/xmldpimp.cxx
+++ b/sc/source/filter/xml/xmldpimp.cxx
@@ -230,7 +230,7 @@ ScXMLDataPilotTableContext::ScXMLDataPilotTableContext( ScXMLImport& rImport,
     }
 
     pDPObject = new ScDPObject(pDoc);
-     pDPSave = new ScDPSaveData();
+    pDPSave = new ScDPSaveData();
 }
 
 ScXMLDataPilotTableContext::~ScXMLDataPilotTableContext()
@@ -295,14 +295,49 @@ SvXMLImportContext *ScXMLDataPilotTableContext::CreateChildContext( sal_uInt16 n
     return pContext;
 }
 
+namespace {
+
+const ScDPSaveDimension* getDimension(
+    const std::vector<const ScDPSaveDimension*>& rRowDims,
+    const std::vector<const ScDPSaveDimension*>& rColDims,
+    ScDPOutputGeometry::FieldType eType, size_t nPos)
+{
+    switch (eType)
+    {
+        case ScDPOutputGeometry::Column:
+        {
+            if (rColDims.size() <= nPos)
+                return NULL;
+
+            return rColDims[nPos];
+        }
+        case ScDPOutputGeometry::Row:
+        {
+            if (rRowDims.size() <= nPos)
+                return NULL;
+
+            return rRowDims[nPos];
+        }
+        default:
+            ;
+    }
+    return NULL;
+}
+
+}
+
 void ScXMLDataPilotTableContext::SetButtons()
 {
-    ScDPOutputGeometry aGeometry(aTargetRangeAddress, bShowFilter, ScDPOutputGeometry::ODF);
+    ScDPOutputGeometry aGeometry(aTargetRangeAddress, bShowFilter);
     aGeometry.setColumnFieldCount(mnColFieldCount);
     aGeometry.setRowFieldCount(mnRowFieldCount);
     aGeometry.setPageFieldCount(mnPageFieldCount);
     aGeometry.setDataFieldCount(mnDataFieldCount);
 
+    std::vector<const ScDPSaveDimension*> aRowDims, aColDims, aPageDims;
+    pDPSave->GetAllDimensionsByOrientation(sheet::DataPilotFieldOrientation_ROW, aRowDims);
+    pDPSave->GetAllDimensionsByOrientation(sheet::DataPilotFieldOrientation_COLUMN, aColDims);
+
     OUString sAddress;
     sal_Int32 nOffset = 0;
     while( nOffset >= 0 )
@@ -314,20 +349,20 @@ void ScXMLDataPilotTableContext::SetButtons()
             sal_Int32 nAddrOffset(0);
             if (pDoc && ScRangeStringConverter::GetAddressFromString( aScAddress, sAddress, pDoc, ::formula::FormulaGrammar::CONV_OOO, nAddrOffset ))
             {
-                ScDPOutputGeometry::FieldType eType = aGeometry.getFieldButtonType(aScAddress);
+                std::pair<ScDPOutputGeometry::FieldType, size_t> aBtnType = aGeometry.getFieldButtonType(aScAddress);
+                const ScDPSaveDimension* pDim = getDimension(
+                    aRowDims, aColDims, aBtnType.first, aBtnType.second);
 
-                sal_Int16 nMFlag = SC_MF_BUTTON;
-                if (eType == ScDPOutputGeometry::Column || eType == ScDPOutputGeometry::Row)
-                    nMFlag |= SC_MF_BUTTON_POPUP;
+                bool bDataLayout = pDim && pDim->IsDataLayout();
+                bool bHasHidden = pDim && pDim->HasInvisibleMember();
 
-                // Use the cell's string value to see if this field contains a
-                // hidden member.  Isn't there a better way?  GetString() is
-                // quite expensive...
-                String aCellStr;
-                pDoc->GetString(aScAddress.Col(), aScAddress.Row(), aScAddress.Tab(), aCellStr);
-                if (maHiddenMemberFields.count(aCellStr))
+                sal_Int16 nMFlag = SC_MF_BUTTON;
+                if (bHasHidden)
                     nMFlag |= SC_MF_HIDDEN_MEMBER;
 
+                if (!bDataLayout)
+                    nMFlag |= SC_MF_BUTTON_POPUP;
+
                 pDoc->ApplyFlagsTab(aScAddress.Col(), aScAddress.Row(), aScAddress.Col(), aScAddress.Row(), aScAddress.Tab(), nMFlag);
             }
         }
@@ -345,40 +380,38 @@ void ScXMLDataPilotTableContext::AddDimension(ScDPSaveDimension* pDim, bool bHas
         //  mark the new one as duplicate
         if ( !pDim->IsDataLayout() &&
                 pDPSave->GetExistingDimensionByName(pDim->GetName()) )
-            pDim->SetDupFlag( sal_True );
+            pDim->SetDupFlag(true);
 
-        if (!pDim->IsDataLayout())
+        switch (pDim->GetOrientation())
         {
-            switch (pDim->GetOrientation())
-            {
-                case sheet::DataPilotFieldOrientation_ROW:
-                    ++mnRowFieldCount;
-                break;
-                case sheet::DataPilotFieldOrientation_COLUMN:
-                    ++mnColFieldCount;
-                break;
-                case sheet::DataPilotFieldOrientation_PAGE:
-                    ++mnPageFieldCount;
-                break;
-                case sheet::DataPilotFieldOrientation_DATA:
-                    ++mnDataFieldCount;
-                break;
-                case sheet::DataPilotFieldOrientation_HIDDEN:
-                default:
-                    ;
-            }
+            case sheet::DataPilotFieldOrientation_ROW:
+                ++mnRowFieldCount;
+            break;
+            case sheet::DataPilotFieldOrientation_COLUMN:
+                ++mnColFieldCount;
+            break;
+            case sheet::DataPilotFieldOrientation_PAGE:
+                ++mnPageFieldCount;
+            break;
+            case sheet::DataPilotFieldOrientation_DATA:
+                ++mnDataFieldCount;
+            break;
+            case sheet::DataPilotFieldOrientation_HIDDEN:
+            default:
+                ;
+        }
 
-            if (bHasHiddenMember)
-            {
-                // the layout name takes priority over the original name,
-                // since this data is used against cell values.
-                const OUString* pLayoutName = pDim->GetLayoutName();
-                if (pLayoutName)
-                    maHiddenMemberFields.insert(*pLayoutName);
-                else
-                    maHiddenMemberFields.insert(pDim->GetName());
-            }
+        if (bHasHiddenMember)
+        {
+            // the layout name takes priority over the original name,
+            // since this data is used against cell values.
+            const OUString* pLayoutName = pDim->GetLayoutName();
+            if (pLayoutName)
+                maHiddenMemberFields.insert(*pLayoutName);
+            else
+                maHiddenMemberFields.insert(pDim->GetName());
         }
+
         pDPSave->AddDimension(pDim);
     }
 }