[Libreoffice-commits] core.git: Branch 'libreoffice-5-4' - sc/source

[Eike Rathke]

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

New commits:
commit 7f208a5287a6eda301b0c3c797d5e21181ce27d2
Author: Eike Rathke <erack at redhat.com>
Date:   Thu Sep 7 22:00:59 2017 +0200

    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 at libreoffice.org>
    Reviewed-by: Caolán McNamara <caolanm at redhat.com>
    Tested-by: Caolán McNamara <caolanm at redhat.com>

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 );