From bece4b6715cfec8eaaa6ee97bade92521d1e2d18 Mon Sep 17 00:00:00 2001
From: Eike Rathke <erack@redhat.com>
Date: Thu, 7 Sep 2017 22:00:59 +0200
Subject: Resolves: tdf#112258 correctly pick items from AutoFilter selection

Combining only one child with one parent to lookup is not sufficient if we have
year,month,day chains. The entire chain is needed.

Regression from

    commit 2a39dc74724d3648ff76aa900edfebe0dd19b296
    Date:   Thu Jun 1 18:27:13 2017 +0200

        tdf#108259 Fix nested checkbox handling in autofilter popup

and

    commit 511fb8e80d298d42f5c45e7410bf64f2a25b441e
    Date:   Wed May 31 10:59:42 2017 +0200

        tdf#108259 Enable autofilter with many different values

which combined landed in 5.4 as

    commit 0163957ef808cffa332c2ddd3267409c5ae1494a
    Date:   Wed May 31 10:59:42 2017 +0200

        tdf#108259 Enable autofilter with many different values

This on the other hand makes the change of these doubtful, as for each leaf
item the concatenation and the lookup has to be done. Something to be
investigated.

Change-Id: I07fd6367bb8da2adab94a43c45fe88391179f496
---
 sc/source/ui/cctrl/checklistmenu.cxx | 41 ++++++++++++++++++++++++++----------
 sc/source/ui/inc/checklistmenu.hxx   |  2 +-
 2 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/sc/source/ui/cctrl/checklistmenu.cxx b/sc/source/ui/cctrl/checklistmenu.cxx
index 15c52b83f1cc..e4f04da83504 100644
--- a/sc/source/ui/cctrl/checklistmenu.cxx
+++ b/sc/source/ui/cctrl/checklistmenu.cxx
@@ -1637,14 +1637,18 @@ void ScCheckListBox::Init()
     SetNodeDefaultImages();
 }
 
-void ScCheckListBox::GetRecursiveChecked(SvTreeListEntry* pEntry, std::unordered_set<OUString, OUStringHash>& vOut, SvTreeListEntry* pParent)
+void ScCheckListBox::GetRecursiveChecked( SvTreeListEntry* pEntry, std::unordered_set<OUString, OUStringHash>& vOut,
+        OUString& rLabel )
 {
     if (GetCheckButtonState(pEntry) == SvButtonState::Checked)
     {
-        // we have to hash both parent and child together
-        OUString aName = GetEntryText(pEntry);
-        if (pParent) aName += GetEntryText(pParent);
-        vOut.insert(aName);
+        // We have to hash parents and children together.
+        // Per convention for easy access in getResult()
+        // "child;parent;grandparent" while descending.
+        if (rLabel.isEmpty())
+            rLabel = GetEntryText(pEntry);
+        else
+            rLabel = GetEntryText(pEntry) + ";" + rLabel;
     }
 
     if (pEntry->HasChildren())
@@ -1652,10 +1656,14 @@ void ScCheckListBox::GetRecursiveChecked(SvTreeListEntry* pEntry, std::unordered
         const SvTreeListEntries& rChildren = pEntry->GetChildEntries();
         for (auto& rChild : rChildren)
         {
-            GetRecursiveChecked(rChild.get(), vOut, pEntry);
+            OUString aLabel = rLabel;
+            GetRecursiveChecked( rChild.get(), vOut, aLabel);
+            if (!aLabel.isEmpty())
+                vOut.insert( aLabel);
         }
+        // Let the caller not add the parent alone.
+        rLabel.clear();
     }
-
 }
 
 std::unordered_set<OUString, OUStringHash> ScCheckListBox::GetAllChecked()
@@ -1665,7 +1673,10 @@ std::unordered_set<OUString, OUStringHash> ScCheckListBox::GetAllChecked()
     SvTreeListEntry* pEntry = GetEntry(nRootPos);
     while (pEntry)
     {
-        GetRecursiveChecked(pEntry, vResults, nullptr);
+        OUString aLabel;
+        GetRecursiveChecked( pEntry, vResults, aLabel);
+        if (!aLabel.isEmpty())
+            vResults.insert( aLabel);
         pEntry = GetEntry(++nRootPos);
     }
 
@@ -1951,9 +1962,17 @@ void ScCheckListMenuWindow::getResult(ResultType& rResult)
             if (aLabel.isEmpty())
                 aLabel = ScGlobal::GetRscString(STR_EMPTYDATA);
 
-            bool bState = vCheckeds.find(maMembers[i].mpParent ?
-                    aLabel.copy(0).concat(maChecks->GetEntryText(maMembers[i].mpParent)) :
-                    aLabel) != vCheckeds.end();
+            /* TODO: performance-wise this looks suspicious, concatenating to
+             * do the lookup for each leaf item seems wasteful. */
+            // Checked labels are in the form "child;parent;grandparent".
+            for (SvTreeListEntry* pParent = maMembers[i].mpParent;
+                    pParent && pParent->GetFirstItem( SvLBoxItemType::String);
+                    pParent = pParent->GetParent())
+            {
+                aLabel += ";" + maChecks->GetEntryText( pParent);
+            }
+            bool bState = vCheckeds.find(aLabel) != vCheckeds.end();
+
             ResultEntry aResultEntry;
             aResultEntry.bValid = bState;
             if ( maMembers[i].mbDate )
diff --git a/sc/source/ui/inc/checklistmenu.hxx b/sc/source/ui/inc/checklistmenu.hxx
index 51771ec6a429..5e7a158e7c70 100644
--- a/sc/source/ui/inc/checklistmenu.hxx
+++ b/sc/source/ui/inc/checklistmenu.hxx
@@ -241,7 +241,7 @@ class ScCheckListBox : public SvTreeListBox
     void CheckEntry( const OUString& sName, SvTreeListEntry* pParent, bool bCheck );
     void CheckEntry( SvTreeListEntry* pEntry, bool bCheck );
     SvTreeListEntry* ShowCheckEntry( const OUString& sName, ScCheckListMember& rMember, bool bShow = true, bool bCheck = true );
-    void GetRecursiveChecked(SvTreeListEntry* pEntry, std::unordered_set<OUString, OUStringHash>& vOut, SvTreeListEntry* pParent);
+    void GetRecursiveChecked( SvTreeListEntry* pEntry, std::unordered_set<OUString, OUStringHash>& vOut, OUString& rLabel );
     std::unordered_set<OUString, OUStringHash> GetAllChecked();
     bool IsChecked( const OUString& sName, SvTreeListEntry* pParent );
     SvTreeListEntry* FindEntry( SvTreeListEntry* pParent, const OUString& sNode );
-- 
cgit