Resolves: tdf#112258 correctly pick items from AutoFilter selection

 This is a combination of 2 commits.

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.

(cherry picked from commit bece4b6715cfec8eaaa6ee97bade92521d1e2d18)

If children then insert only if a child is selected, tdf#112258 follow-up

Otherwise the chain down to the parent would be pickable, though in practice it
doesn't have a consequence as only leaves are picked. However, inserting to the
set can be avoided.

(cherry picked from commit fd5ac4c4f5de9ef475156dafcba1b37a56b5cb8d)

6a6b63b2d7c031a2dd13d8cc7d98f15e1f946298

Change-Id: I07fd6367bb8da2adab94a43c45fe88391179f496
Reviewed-on: https://gerrit.libreoffice.org/42085
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Tested-by: Caolán McNamara <caolanm@redhat.com>

2 files changed, 31 insertions, 12 deletions

diff --git a/sc/source/ui/cctrl/checklistmenu.cxx b/sc/source/ui/cctrl/checklistmenu.cxx
index 8b43361e8ced..c6657cbab41e 100644
--- a/sc/source/ui/cctrl/checklistmenu.cxx
+++ b/sc/source/ui/cctrl/checklistmenu.cxx
@@ -1639,14 +1639,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())
@@ -1654,10 +1658,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() && aLabel != rLabel)
+                vOut.insert( aLabel);
         }
+        // Let the caller not add the parent alone.
+        rLabel.clear();
     }
-
 }
 
 std::unordered_set<OUString, OUStringHash> ScCheckListBox::GetAllChecked()
@@ -1667,7 +1675,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);
     }
 
@@ -1952,9 +1963,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 6ac7c4879fc2..df6871c2eeb3 100644
--- a/sc/source/ui/inc/checklistmenu.hxx
+++ b/sc/source/ui/inc/checklistmenu.hxx
@@ -240,7 +240,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 );