tdf#76258 Various fixes for color filter

Change-Id: Ifb4114597efcb7a957d2e42867d1bdc8abfe7730
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/115340
Tested-by: Jenkins
Reviewed-by: Samuel Mehrbrodt <samuel.mehrbrodt@allotropia.de>

5 files changed, 43 insertions, 26 deletions

diff --git a/sc/source/core/data/column3.cxx b/sc/source/core/data/column3.cxx
index 5fd4a5965586..eef630c6f297 100644
--- a/sc/source/core/data/column3.cxx
+++ b/sc/source/core/data/column3.cxx
@@ -2432,12 +2432,12 @@ class FilterEntriesHandler
 
         const SvxBrushItem* pBrush = rColumn.GetDoc().GetAttr(aPos, ATTR_BACKGROUND);
         Color backgroundColor = pBrush->GetColor();
+        mrFilterEntries.addTextColor(textColor);
+        mrFilterEntries.addBackgroundColor(backgroundColor);
 
         if (rCell.hasString())
         {
             mrFilterEntries.push_back(ScTypedStrData(aStr));
-            mrFilterEntries.addTextColor(textColor);
-            mrFilterEntries.addBackgroundColor(backgroundColor);
             return;
         }
 
@@ -2460,8 +2460,6 @@ class FilterEntriesHandler
                     if (!aErr.isEmpty())
                     {
                         mrFilterEntries.push_back(ScTypedStrData(aErr));
-                        mrFilterEntries.addTextColor(textColor);
-                        mrFilterEntries.addBackgroundColor(backgroundColor);
                         return;
                     }
                 }
@@ -2499,8 +2497,6 @@ class FilterEntriesHandler
         to avoid duplicates in the filter lists with setting the mbIsFormatted */
         bool bFormFiltVal = mrColumn.HasFiltering() && nFormat;
         mrFilterEntries.push_back(ScTypedStrData(aStr, fVal, ScTypedStrData::Value, bDate, bFormFiltVal));
-        mrFilterEntries.addTextColor(textColor);
-        mrFilterEntries.addBackgroundColor(backgroundColor);
     }
 
 public:
diff --git a/sc/source/core/data/table3.cxx b/sc/source/core/data/table3.cxx
index dcc76104280b..54c0850931ee 100644
--- a/sc/source/core/data/table3.cxx
+++ b/sc/source/core/data/table3.cxx
@@ -2817,31 +2817,31 @@ bool ScTable::ValidQuery(
         {
             for (const auto& rItem : rItems)
             {
-                if (aEval.isQueryByValue(rItem, nCol, nRow, aCell))
+                if (rItem.meType == ScQueryEntry::ByTextColor)
                 {
-                    std::pair<bool,bool> aThisRes =
-                        aEval.compareByValue(aCell, nCol, nRow, rEntry, rItem, pContext);
+                    std::pair<bool, bool> aThisRes
+                        = aEval.compareByTextColor(nCol, nRow, nTab, rItem);
                     aRes.first |= aThisRes.first;
                     aRes.second |= aThisRes.second;
                 }
-                else if (aEval.isQueryByString(rEntry, rItem, nCol, nRow, aCell))
+                else if (rItem.meType == ScQueryEntry::ByBackgroundColor)
                 {
                     std::pair<bool,bool> aThisRes =
-                        aEval.compareByString(aCell, nRow, rEntry, rItem, pContext);
+                        aEval.compareByBackgroundColor(nCol, nRow, nTab, rItem);
                     aRes.first |= aThisRes.first;
                     aRes.second |= aThisRes.second;
                 }
-                if (rItem.meType == ScQueryEntry::ByTextColor)
+                else if (aEval.isQueryByValue(rItem, nCol, nRow, aCell))
                 {
-                    std::pair<bool, bool> aThisRes
-                        = aEval.compareByTextColor(nCol, nRow, nTab, rItem);
+                    std::pair<bool,bool> aThisRes =
+                        aEval.compareByValue(aCell, nCol, nRow, rEntry, rItem, pContext);
                     aRes.first |= aThisRes.first;
                     aRes.second |= aThisRes.second;
                 }
-                if (rItem.meType == ScQueryEntry::ByBackgroundColor)
+                else if (aEval.isQueryByString(rEntry, rItem, nCol, nRow, aCell))
                 {
                     std::pair<bool,bool> aThisRes =
-                        aEval.compareByBackgroundColor(nCol, nRow, nTab, rItem);
+                        aEval.compareByString(aCell, nRow, rEntry, rItem, pContext);
                     aRes.first |= aThisRes.first;
                     aRes.second |= aThisRes.second;
                 }
diff --git a/sc/source/ui/cctrl/checklistmenu.cxx b/sc/source/ui/cctrl/checklistmenu.cxx
index a7ae8ddbd960..1e8cb5662d82 100644
--- a/sc/source/ui/cctrl/checklistmenu.cxx
+++ b/sc/source/ui/cctrl/checklistmenu.cxx
@@ -198,8 +198,6 @@ void ScCheckListMenuControl::executeMenuItem(size_t nPos)
         return;
 
     maMenuItems[nPos].mxAction->execute();
-
-    terminateAllPopupMenus();
 }
 
 void ScCheckListMenuControl::setSelectedMenuItem(size_t nPos, bool bSubMenuTimer)
diff --git a/sc/source/ui/inc/checklistmenu.hxx b/sc/source/ui/inc/checklistmenu.hxx
index a97f519d1fd7..363de1b51a78 100644
--- a/sc/source/ui/inc/checklistmenu.hxx
+++ b/sc/source/ui/inc/checklistmenu.hxx
@@ -173,6 +173,12 @@ public:
     int GetTextWidth(const OUString& rsName) const;
     int IncreaseWindowWidthToFitText(int nMaxTextWidth);
 
+    /**
+     * Dismiss all visible popup menus and set focus back to the application
+     * window.  This method is called e.g. when a menu action is fired.
+     */
+    void terminateAllPopupMenus();
+
 private:
 
     std::vector<MenuItemData>         maMenuItems;
@@ -200,12 +206,6 @@ private:
 
     void executeMenuItem(size_t nPos);
 
-    /**
-     * Dismiss all visible popup menus and set focus back to the application
-     * window.  This method is called e.g. when a menu action is fired.
-     */
-    void terminateAllPopupMenus();
-
     void endSubMenu(ScCheckListMenuControl& rSubMenu);
 
     struct SubMenuItemData;
diff --git a/sc/source/ui/view/gridwin.cxx b/sc/source/ui/view/gridwin.cxx
index 1580640dd40a..c6c1f5250bf3 100644
--- a/sc/source/ui/view/gridwin.cxx
+++ b/sc/source/ui/view/gridwin.cxx
@@ -745,6 +745,11 @@ void ScGridWindow::UpdateAutoFilterFromMenu(AutoFilterMode eMode)
 {
     ScCheckListMenuControl& rControl = mpAutoFilterPopup->get_widget();
 
+    // Terminate autofilter popup now when there is no further user input needed
+    bool bColorMode = eMode == AutoFilterMode::TextColor || eMode == AutoFilterMode::BackgroundColor;
+    if (!bColorMode)
+        rControl.terminateAllPopupMenus();
+
     const AutoFilterData* pData =
         static_cast<const AutoFilterData*>(rControl.getExtendedData());
 
@@ -833,7 +838,8 @@ void ScGridWindow::UpdateAutoFilterFromMenu(AutoFilterMode eMode)
     }
 
     // Remove old entries in auto-filter rules
-    aParam.RemoveAllEntriesByField(rPos.Col());
+    if (!bColorMode)
+        aParam.RemoveAllEntriesByField(rPos.Col());
 
     if( !(eMode == AutoFilterMode::Normal && rControl.isAllSelected() ) )
     {
@@ -886,19 +892,36 @@ void ScGridWindow::UpdateAutoFilterFromMenu(AutoFilterMode eMode)
                 std::set<Color> aColors = eMode == AutoFilterMode::TextColor
                                               ? aFilterEntries.getTextColors()
                                               : aFilterEntries.getBackgroundColors();
+
                 sal_Int32 i = 1;
+                sal_Int32 nActive = -1;
                 for (auto& rColor : aColors)
                 {
                     pColorMenu->InsertItem(i, OUString(), MenuItemBits::CHECKABLE);
                     pColorMenu->SetItemColor(i, rColor);
+                    auto aItem = pEntry->GetQueryItem();
+                    if (aItem.maColor == rColor
+                        && ((eMode == AutoFilterMode::TextColor
+                             && aItem.meType == ScQueryEntry::ByTextColor)
+                            || (eMode == AutoFilterMode::BackgroundColor
+                                && aItem.meType == ScQueryEntry::ByBackgroundColor)))
+                    {
+                        nActive = i;
+                        pColorMenu->CheckItem(i, true);
+                    }
                     i++;
                 }
 
                 sal_uInt16 nSelected = pColorMenu->Execute(this, mpAutoFilterPopup->GetPosPixel());
                 pColorMenu.disposeAndClear();
+                rControl.terminateAllPopupMenus();
 
                 if (nSelected == 0)
-                    break;
+                    return;
+
+                // Disable color filter when active color was selected
+                if (nSelected == nActive)
+                    aParam.RemoveAllEntriesByField(rPos.Col());
 
                 // Get selected color from set
                 std::set<Color>::iterator it = aColors.begin();