tdf#122527 dot symbols in chart menubutton not working

a few problems here

*) each symbol was drawn on top of the previous one instead of clearing it
*) some spurious placeholder text in the menu entry
*) m_nSymbolType not set correctly on activation of a submenu entry
*) under gtk3 adding entries to the submenus didn't inform the
menubutton of their addition
*) we can drop m_xMenu because set_item_sensitive can be used
instead, which didn't exist at the initial time of writing

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

3 files changed, 193 insertions, 148 deletions

diff --git a/cui/source/inc/cuitabline.hxx b/cui/source/inc/cuitabline.hxx
index 639ad645c7ce..284a0e656a55 100644
--- a/cui/source/inc/cuitabline.hxx
+++ b/cui/source/inc/cuitabline.hxx
@@ -148,7 +148,6 @@ private:
     std::unique_ptr<weld::MetricSpinButton> m_xMtrEndWidth;
     std::unique_ptr<weld::CheckButton> m_xTsbCenterEnd;
     std::unique_ptr<weld::CheckButton> m_xCbxSynchronize;
-    std::unique_ptr<weld::Menu> m_xMenu;
     std::unique_ptr<weld::Menu> m_xGalleryMenu;
     std::unique_ptr<weld::Menu> m_xSymbolsMenu;
     std::unique_ptr<weld::CustomWeld> m_xCtlPreview;
diff --git a/cui/source/tabpages/tpline.cxx b/cui/source/tabpages/tpline.cxx
index dad85068f793..b4498348c8ed 100644
--- a/cui/source/tabpages/tpline.cxx
+++ b/cui/source/tabpages/tpline.cxx
@@ -108,7 +108,6 @@ SvxLineTabPage::SvxLineTabPage(TabPageParent pParent, const SfxItemSet& rInAttrs
     , m_xMtrEndWidth(m_xBuilder->weld_metric_spin_button("MTR_FLD_END_WIDTH", FieldUnit::CM))
     , m_xTsbCenterEnd(m_xBuilder->weld_check_button("TSB_CENTER_END"))
     , m_xCbxSynchronize(m_xBuilder->weld_check_button("CBX_SYNCHRONIZE"))
-    , m_xMenu(m_xBuilder->weld_menu("menuSELECT"))
     , m_xCtlPreview(new weld::CustomWeld(*m_xBuilder, "CTL_PREVIEW", m_aCtlPreview))
     , m_xFLEdgeStyle(m_xBuilder->weld_widget("FL_EDGE_STYLE"))
     , m_xGridEdgeCaps(m_xBuilder->weld_widget("gridEDGE_CAPS"))
@@ -1410,7 +1409,7 @@ IMPL_LINK_NOARG(SvxLineTabPage, MenuCreateHdl_Impl, weld::ToggleButton&, void)
                     aBitmap.Scale(nScale, nScale);
 
                 }
-                pVD->SetOutputSizePixel(aBitmap.GetSizePixel(), false);
+                pVD->SetOutputSizePixel(aBitmap.GetSizePixel());
                 pVD->DrawBitmapEx(Point(), aBitmap);
                 m_xGalleryMenu->append(pInfo->sItemId, *pUIName, *pVD);
             }
@@ -1422,7 +1421,7 @@ IMPL_LINK_NOARG(SvxLineTabPage, MenuCreateHdl_Impl, weld::ToggleButton&, void)
         }
 
         if (m_aGrfNames.empty())
-            m_xMenu->set_sensitive("gallery", false);
+            m_xSymbolMB->set_item_sensitive("gallery", false);
     }
 
     if (!m_xSymbolsMenu && m_pSymbolList)
@@ -1494,15 +1493,15 @@ IMPL_LINK_NOARG(SvxLineTabPage, MenuCreateHdl_Impl, weld::ToggleButton&, void)
                                         double(MAX_BMP_HEIGHT) / static_cast<double>(aSize.Height());
                     aBitmapEx.Scale(nScale, nScale);
                 }
-                pVD->SetOutputSizePixel(aBitmapEx.GetSizePixel(), false);
+                pVD->SetOutputSizePixel(aBitmapEx.GetSizePixel());
                 pVD->DrawBitmapEx(Point(), aBitmapEx);
-                m_xSymbolsMenu->append(pInfo->sItemId, "foo", *pVD);
+                m_xSymbolsMenu->append(pInfo->sItemId, "", *pVD);
             }
             pInvisibleSquare=pPage->RemoveObject(0);
             SdrObject::Free(pInvisibleSquare);
 
             if (m_aGrfNames.empty())
-                m_xMenu->set_sensitive("symbols", false);
+                m_xSymbolMB->set_item_sensitive("symbols", false);
         }
     }
 }
@@ -1522,10 +1521,12 @@ IMPL_LINK(SvxLineTabPage, GraphicHdl_Impl, const OString&, rIdent, void)
     {
         SvxBmpItemInfo* pInfo = m_aGalleryBrushItems[sNumber.toUInt32()].get();
         pGraphic = pInfo->pBrushItem->GetGraphic();
+        m_nSymbolType = SVX_SYMBOLTYPE_BRUSHITEM;
     }
     else if (rIdent.startsWith("symbol", &sNumber))
     {
-        SvxBmpItemInfo* pInfo = m_aSymbolBrushItems[sNumber.toUInt32()].get();
+        m_nSymbolType = sNumber.toUInt32();
+        SvxBmpItemInfo* pInfo = m_aSymbolBrushItems[m_nSymbolType].get();
         pGraphic = pInfo->pBrushItem->GetGraphic();
     }
     else if (rIdent == "automatic")
@@ -1559,7 +1560,7 @@ IMPL_LINK(SvxLineTabPage, GraphicHdl_Impl, const OString&, rIdent, void)
             return;
     }
 
-    if(pGraphic)
+    if (pGraphic)
     {
         Size aSize = SvxNumberFormat::GetGraphicSizeMM100(pGraphic);
         aSize = OutputDevice::LogicToLogic(aSize, MapMode(MapUnit::Map100thMM), MapMode(m_ePoolUnit));
diff --git a/vcl/unx/gtk3/gtk3gtkinst.cxx b/vcl/unx/gtk3/gtk3gtkinst.cxx
index b9155e2da687..763fd1efe368 100644
--- a/vcl/unx/gtk3/gtk3gtkinst.cxx
+++ b/vcl/unx/gtk3/gtk3gtkinst.cxx
@@ -1891,145 +1891,6 @@ public:
     }
 };
 
-class GtkInstanceMenu : public MenuHelper, public virtual weld::Menu
-{
-protected:
-    OString m_sActivated;
-
-private:
-    virtual void signal_activate(GtkMenuItem* pItem) override
-    {
-        const gchar* pStr = gtk_buildable_get_name(GTK_BUILDABLE(pItem));
-        m_sActivated = OString(pStr, pStr ? strlen(pStr) : 0);
-    }
-
-public:
-    GtkInstanceMenu(GtkMenu* pMenu, bool bTakeOwnership)
-        : MenuHelper(pMenu, bTakeOwnership)
-    {
-    }
-
-    virtual OString popup_at_rect(weld::Widget* pParent, const tools::Rectangle &rRect) override
-    {
-        m_sActivated.clear();
-
-        GtkInstanceWidget* pGtkWidget = dynamic_cast<GtkInstanceWidget*>(pParent);
-        assert(pGtkWidget);
-
-        GtkWidget* pWidget = pGtkWidget->getWidget();
-        gtk_menu_attach_to_widget(m_pMenu, pWidget, nullptr);
-
-        //run in a sub main loop because we need to keep vcl PopupMenu alive to use
-        //it during DispatchCommand, returning now to the outer loop causes the
-        //launching PopupMenu to be destroyed, instead run the subloop here
-        //until the gtk menu is destroyed
-        GMainLoop* pLoop = g_main_loop_new(nullptr, true);
-        gulong nSignalId = g_signal_connect_swapped(G_OBJECT(m_pMenu), "deactivate", G_CALLBACK(g_main_loop_quit), pLoop);
-
-#if GTK_CHECK_VERSION(3,22,0)
-        if (gtk_check_version(3, 22, 0) == nullptr)
-        {
-            GdkRectangle aRect{static_cast<int>(rRect.Left()), static_cast<int>(rRect.Top()),
-                               static_cast<int>(rRect.GetWidth()), static_cast<int>(rRect.GetHeight())};
-            if (AllSettings::GetLayoutRTL())
-                aRect.x = gtk_widget_get_allocated_width(pWidget) - aRect.width - 1 - aRect.x;
-            gtk_menu_popup_at_rect(m_pMenu, gtk_widget_get_window(pWidget), &aRect, GDK_GRAVITY_NORTH_WEST, GDK_GRAVITY_NORTH_WEST, nullptr);
-        }
-        else
-#else
-        (void) rRect;
-#endif
-        {
-            guint nButton;
-            guint32 nTime;
-
-            //typically there is an event, and we can then distinguish if this was
-            //launched from the keyboard (gets auto-mnemoniced) or the mouse (which
-            //doesn't)
-            GdkEvent *pEvent = gtk_get_current_event();
-            if (pEvent)
-            {
-                gdk_event_get_button(pEvent, &nButton);
-                nTime = gdk_event_get_time(pEvent);
-            }
-            else
-            {
-                nButton = 0;
-                nTime = GtkSalFrame::GetLastInputEventTime();
-            }
-
-            gtk_menu_popup(m_pMenu, nullptr, nullptr, nullptr, nullptr, nButton, nTime);
-        }
-
-        if (g_main_loop_is_running(pLoop))
-        {
-            gdk_threads_leave();
-            g_main_loop_run(pLoop);
-            gdk_threads_enter();
-        }
-        g_main_loop_unref(pLoop);
-        g_signal_handler_disconnect(m_pMenu, nSignalId);
-
-        return m_sActivated;
-    }
-
-    virtual void set_sensitive(const OString& rIdent, bool bSensitive) override
-    {
-        set_item_sensitive(rIdent, bSensitive);
-    }
-
-    virtual void set_active(const OString& rIdent, bool bActive) override
-    {
-        set_item_active(rIdent, bActive);
-    }
-
-    virtual void show(const OString& rIdent, bool bShow) override
-    {
-        show_item(rIdent, bShow);
-    }
-
-    virtual void insert(int pos, const OUString& rId, const OUString& rStr,
-                        const OUString* pIconName, VirtualDevice* pImageSufface,
-                        bool bCheck) override
-    {
-        GtkWidget* pImage = nullptr;
-        if (pIconName)
-        {
-            GdkPixbuf* pixbuf = load_icon_by_name(*pIconName);
-            if (!pixbuf)
-            {
-                pImage = gtk_image_new_from_pixbuf(pixbuf);
-                g_object_unref(pixbuf);
-            }
-        }
-        else if (pImageSufface)
-            pImage = gtk_image_new_from_surface(get_underlying_cairo_surface(*pImageSufface));
-
-        GtkWidget *pItem;
-        if (pImage)
-        {
-            GtkWidget *pBox = gtk_box_new(GTK_ORIENTATION_HORIZONTAL, 6);
-            GtkWidget *pLabel = gtk_label_new(MapToGtkAccelerator(rStr).getStr());
-            pItem = bCheck ? gtk_check_menu_item_new() : gtk_menu_item_new();
-            gtk_container_add(GTK_CONTAINER(pBox), pImage);
-            gtk_container_add(GTK_CONTAINER(pBox), pLabel);
-            gtk_container_add(GTK_CONTAINER(pItem), pBox);
-            gtk_widget_show_all(pItem);
-        }
-        else
-        {
-            pItem = bCheck ? gtk_check_menu_item_new_with_label(MapToGtkAccelerator(rStr).getStr())
-                           : gtk_menu_item_new_with_label(MapToGtkAccelerator(rStr).getStr());
-        }
-        gtk_buildable_set_name(GTK_BUILDABLE(pItem), OUStringToOString(rId, RTL_TEXTENCODING_UTF8).getStr());
-        gtk_menu_shell_append(GTK_MENU_SHELL(m_pMenu), pItem);
-        gtk_widget_show(pItem);
-        add_to_map(GTK_MENU_ITEM(pItem));
-        if (pos != -1)
-            gtk_menu_reorder_child(m_pMenu, pItem, pos);
-    }
-};
-
 class GtkInstanceSizeGroup : public weld::SizeGroup
 {
 private:
@@ -4166,6 +4027,190 @@ public:
     }
 };
 
+class GtkInstanceMenu : public MenuHelper, public virtual weld::Menu
+{
+protected:
+    OString m_sActivated;
+    GtkInstanceMenuButton* m_pTopLevelMenuButton;
+
+private:
+    virtual void signal_activate(GtkMenuItem* pItem) override
+    {
+        const gchar* pStr = gtk_buildable_get_name(GTK_BUILDABLE(pItem));
+        m_sActivated = OString(pStr, pStr ? strlen(pStr) : 0);
+    }
+
+public:
+    GtkInstanceMenu(GtkMenu* pMenu, bool bTakeOwnership)
+        : MenuHelper(pMenu, bTakeOwnership)
+        , m_pTopLevelMenuButton(nullptr)
+    {
+        // tdf#122527 if we're welding a submenu of a menu of a MenuButton,
+        // then find that MenuButton parent so that when adding items to this
+        // menu we can inform the MenuButton of their addition
+        GtkMenu* pTopLevelMenu = pMenu;
+        while (true)
+        {
+            GtkWidget* pAttached = gtk_menu_get_attach_widget(pTopLevelMenu);
+            if (!pAttached || !GTK_IS_MENU_ITEM(pAttached))
+                break;
+            GtkWidget* pParent = gtk_widget_get_parent(pAttached);
+            if (!pParent || !GTK_IS_MENU(pParent))
+                break;
+            pTopLevelMenu = GTK_MENU(pParent);
+        }
+        if (pTopLevelMenu != pMenu)
+        {
+            GtkWidget* pAttached = gtk_menu_get_attach_widget(pTopLevelMenu);
+            if (pAttached && GTK_IS_MENU_BUTTON(pAttached))
+            {
+                void* pData = g_object_get_data(G_OBJECT(pAttached), "g-lo-GtkInstanceButton");
+                m_pTopLevelMenuButton = dynamic_cast<GtkInstanceMenuButton*>(static_cast<GtkInstanceButton*>(pData));
+            }
+        }
+    }
+
+    virtual OString popup_at_rect(weld::Widget* pParent, const tools::Rectangle &rRect) override
+    {
+        m_sActivated.clear();
+
+        GtkInstanceWidget* pGtkWidget = dynamic_cast<GtkInstanceWidget*>(pParent);
+        assert(pGtkWidget);
+
+        GtkWidget* pWidget = pGtkWidget->getWidget();
+        gtk_menu_attach_to_widget(m_pMenu, pWidget, nullptr);
+
+        //run in a sub main loop because we need to keep vcl PopupMenu alive to use
+        //it during DispatchCommand, returning now to the outer loop causes the
+        //launching PopupMenu to be destroyed, instead run the subloop here
+        //until the gtk menu is destroyed
+        GMainLoop* pLoop = g_main_loop_new(nullptr, true);
+        gulong nSignalId = g_signal_connect_swapped(G_OBJECT(m_pMenu), "deactivate", G_CALLBACK(g_main_loop_quit), pLoop);
+
+#if GTK_CHECK_VERSION(3,22,0)
+        if (gtk_check_version(3, 22, 0) == nullptr)
+        {
+            GdkRectangle aRect{static_cast<int>(rRect.Left()), static_cast<int>(rRect.Top()),
+                               static_cast<int>(rRect.GetWidth()), static_cast<int>(rRect.GetHeight())};
+            if (AllSettings::GetLayoutRTL())
+                aRect.x = gtk_widget_get_allocated_width(pWidget) - aRect.width - 1 - aRect.x;
+            gtk_menu_popup_at_rect(m_pMenu, gtk_widget_get_window(pWidget), &aRect, GDK_GRAVITY_NORTH_WEST, GDK_GRAVITY_NORTH_WEST, nullptr);
+        }
+        else
+#else
+        (void) rRect;
+#endif
+        {
+            guint nButton;
+            guint32 nTime;
+
+            //typically there is an event, and we can then distinguish if this was
+            //launched from the keyboard (gets auto-mnemoniced) or the mouse (which
+            //doesn't)
+            GdkEvent *pEvent = gtk_get_current_event();
+            if (pEvent)
+            {
+                gdk_event_get_button(pEvent, &nButton);
+                nTime = gdk_event_get_time(pEvent);
+            }
+            else
+            {
+                nButton = 0;
+                nTime = GtkSalFrame::GetLastInputEventTime();
+            }
+
+            gtk_menu_popup(m_pMenu, nullptr, nullptr, nullptr, nullptr, nButton, nTime);
+        }
+
+        if (g_main_loop_is_running(pLoop))
+        {
+            gdk_threads_leave();
+            g_main_loop_run(pLoop);
+            gdk_threads_enter();
+        }
+        g_main_loop_unref(pLoop);
+        g_signal_handler_disconnect(m_pMenu, nSignalId);
+
+        return m_sActivated;
+    }
+
+    virtual void set_sensitive(const OString& rIdent, bool bSensitive) override
+    {
+        set_item_sensitive(rIdent, bSensitive);
+    }
+
+    virtual void set_active(const OString& rIdent, bool bActive) override
+    {
+        set_item_active(rIdent, bActive);
+    }
+
+    virtual void show(const OString& rIdent, bool bShow) override
+    {
+        show_item(rIdent, bShow);
+    }
+
+    virtual void insert(int pos, const OUString& rId, const OUString& rStr,
+                        const OUString* pIconName, VirtualDevice* pImageSufface,
+                        bool bCheck) override
+    {
+        GtkWidget* pImage = nullptr;
+        if (pIconName)
+        {
+            GdkPixbuf* pixbuf = load_icon_by_name(*pIconName);
+            if (!pixbuf)
+            {
+                pImage = gtk_image_new_from_pixbuf(pixbuf);
+                g_object_unref(pixbuf);
+            }
+        }
+        else if (pImageSufface)
+        {
+            cairo_surface_t* surface = get_underlying_cairo_surface(*pImageSufface);
+
+            Size aSize(pImageSufface->GetOutputSizePixel());
+            cairo_surface_t* target = cairo_surface_create_similar(surface,
+                                                                    cairo_surface_get_content(surface),
+                                                                    aSize.Width(),
+                                                                    aSize.Height());
+
+            cairo_t* cr = cairo_create(target);
+            cairo_set_source_surface(cr, surface, 0, 0);
+            cairo_paint(cr);
+            cairo_destroy(cr);
+
+            pImage = gtk_image_new_from_surface(target);
+
+            cairo_surface_destroy(target);
+        }
+
+        GtkWidget *pItem;
+        if (pImage)
+        {
+            GtkWidget *pBox = gtk_box_new(GTK_ORIENTATION_HORIZONTAL, 6);
+            GtkWidget *pLabel = gtk_label_new(MapToGtkAccelerator(rStr).getStr());
+            pItem = bCheck ? gtk_check_menu_item_new() : gtk_menu_item_new();
+            gtk_container_add(GTK_CONTAINER(pBox), pImage);
+            gtk_container_add(GTK_CONTAINER(pBox), pLabel);
+            gtk_container_add(GTK_CONTAINER(pItem), pBox);
+            gtk_widget_show_all(pItem);
+        }
+        else
+        {
+            pItem = bCheck ? gtk_check_menu_item_new_with_label(MapToGtkAccelerator(rStr).getStr())
+                           : gtk_menu_item_new_with_label(MapToGtkAccelerator(rStr).getStr());
+        }
+        gtk_buildable_set_name(GTK_BUILDABLE(pItem), OUStringToOString(rId, RTL_TEXTENCODING_UTF8).getStr());
+        gtk_menu_shell_append(GTK_MENU_SHELL(m_pMenu), pItem);
+        gtk_widget_show(pItem);
+        GtkMenuItem* pMenuItem = GTK_MENU_ITEM(pItem);
+        add_to_map(pMenuItem);
+        if (m_pTopLevelMenuButton)
+            m_pTopLevelMenuButton->add_to_map(pMenuItem);
+        if (pos != -1)
+            gtk_menu_reorder_child(m_pMenu, pItem, pos);
+    }
+};
+
 class GtkInstanceRadioButton : public GtkInstanceToggleButton, public virtual weld::RadioButton
 {
 public: