jsdialog: use string identifiers

Thanks to that it is easier to debug and find widgets in maps.
Sidebar and notebookbar are now in different map entries.
This fixes the issue when destroying notebookbar deleted sidebar's
widgets.
After this patch sidebar works correctly when using notebookbar mode in
lok.

Change-Id: Ie9dcb82675129bdb567b766e29779744f500cb48
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/125216
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Szymon Kłos <szymon.klos@collabora.com>

7 files changed, 62 insertions, 38 deletions

diff --git a/desktop/source/lib/init.cxx b/desktop/source/lib/init.cxx
index e71e62929903..feed3452dcf1 100644
--- a/desktop/source/lib/init.cxx
+++ b/desktop/source/lib/init.cxx
@@ -4101,15 +4101,23 @@ static void lcl_sendDialogEvent(unsigned long long int nWindowId, const char* pA
         OString sControlId = OUStringToOString(aMap["id"], RTL_TEXTENCODING_ASCII_US);
 
         // dialogs send own id but notebookbar and sidebar controls are remembered by SfxViewShell id
-        bool bFoundWeldedControl = jsdialog::ExecuteAction(nWindowId, sControlId, aMap);
+        bool bFoundWeldedControl = jsdialog::ExecuteAction(std::to_string(nWindowId), sControlId, aMap);
         if (!bFoundWeldedControl)
-            bFoundWeldedControl = jsdialog::ExecuteAction(nCurrentShellId, sControlId, aMap);
+            bFoundWeldedControl = jsdialog::ExecuteAction(std::to_string(nCurrentShellId) + "sidebar", sControlId, aMap);
+        if (!bFoundWeldedControl)
+            bFoundWeldedControl = jsdialog::ExecuteAction(std::to_string(nCurrentShellId) + "notebookbar", sControlId, aMap);
+        if (!bFoundWeldedControl && !SfxViewShell::Current())
+        {
+            // this is needed for dialogs shown before document is loaded: MacroWarning dialog, etc...
+            // these dialogs are created with WindowId "0"
+            bFoundWeldedControl = jsdialog::ExecuteAction("0", sControlId, aMap);
+        }
 
         if (bFoundWeldedControl)
             return;
 
         // force resend - used in mobile-wizard
-        jsdialog::SendFullUpdate(nCurrentShellId, "Panel");
+        jsdialog::SendFullUpdate(std::to_string(nCurrentShellId) + "sidebar", "Panel");
 
     } catch(...) {}
 }
diff --git a/include/vcl/jsdialog/executor.hxx b/include/vcl/jsdialog/executor.hxx
index 46455e985735..39c8c77f3a24 100644
--- a/include/vcl/jsdialog/executor.hxx
+++ b/include/vcl/jsdialog/executor.hxx
@@ -60,8 +60,9 @@ public:
 
 namespace jsdialog
 {
-VCL_DLLPUBLIC bool ExecuteAction(sal_uInt64 nWindowId, const OString& rWidget, StringMap& rData);
-VCL_DLLPUBLIC void SendFullUpdate(sal_uInt64 nWindowId, const OString& rWidget);
+VCL_DLLPUBLIC bool ExecuteAction(const std::string& nWindowId, const OString& rWidget,
+                                 StringMap& rData);
+VCL_DLLPUBLIC void SendFullUpdate(const std::string& nWindowId, const OString& rWidget);
 VCL_DLLPUBLIC StringMap jsonToStringMap(const char* pJSON);
 };
 
diff --git a/sc/source/ui/cctrl/checklistmenu.cxx b/sc/source/ui/cctrl/checklistmenu.cxx
index 54c87adad219..efd8688dc649 100644
--- a/sc/source/ui/cctrl/checklistmenu.cxx
+++ b/sc/source/ui/cctrl/checklistmenu.cxx
@@ -300,7 +300,7 @@ void ScCheckListMenuControl::launchSubMenu(bool bSetMenuPos)
     rSubMenuControl.GrabFocus();
 
     if (comphelper::LibreOfficeKit::isActive())
-        jsdialog::SendFullUpdate(pSubMenu->GetLOKWindowId(), "toggle_all");
+        jsdialog::SendFullUpdate(std::to_string(pSubMenu->GetLOKWindowId()), "toggle_all");
 }
 
 IMPL_LINK_NOARG(ScCheckListMenuControl, PostPopdownHdl, void*, void)
diff --git a/sfx2/source/sidebar/DeckLayouter.cxx b/sfx2/source/sidebar/DeckLayouter.cxx
index 9a758f1f7215..b1ae79129d13 100644
--- a/sfx2/source/sidebar/DeckLayouter.cxx
+++ b/sfx2/source/sidebar/DeckLayouter.cxx
@@ -312,7 +312,10 @@ sal_Int32 PlacePanels (
     }
 
     if (comphelper::LibreOfficeKit::isActive())
-        jsdialog::SendFullUpdate(reinterpret_cast<sal_uInt64>(SfxViewShell::Current()), "Panel");
+    {
+        sal_uInt64 nShellId = reinterpret_cast<sal_uInt64>(SfxViewShell::Current());
+        jsdialog::SendFullUpdate(std::to_string(nShellId) + "sidebar", "Panel");
+    }
 
     return nY;
 }
diff --git a/vcl/inc/jsdialog/jsdialogbuilder.hxx b/vcl/inc/jsdialog/jsdialogbuilder.hxx
index 9f52846206d7..ccd2489d797b 100644
--- a/vcl/inc/jsdialog/jsdialogbuilder.hxx
+++ b/vcl/inc/jsdialog/jsdialogbuilder.hxx
@@ -209,15 +209,17 @@ class JSInstanceBuilder : public SalInstanceBuilder, public JSDialogSender
     /// When LOKNotifier is set by jsdialogs code we need to release it
     VclPtr<vcl::Window> m_aWindowToRelease;
 
-    friend VCL_DLLPUBLIC bool jsdialog::ExecuteAction(sal_uInt64 nWindowId, const OString& rWidget,
-                                                      StringMap& rData);
-    friend VCL_DLLPUBLIC void jsdialog::SendFullUpdate(sal_uInt64 nWindowId,
+    friend VCL_DLLPUBLIC bool jsdialog::ExecuteAction(const std::string& nWindowId,
+                                                      const OString& rWidget, StringMap& rData);
+    friend VCL_DLLPUBLIC void jsdialog::SendFullUpdate(const std::string& nWindowId,
                                                        const OString& rWidget);
 
-    static std::map<sal_uInt64, WidgetMap>& GetLOKWeldWidgetsMap();
-    static void InsertWindowToMap(sal_uInt64 nWindowId);
+    static std::map<std::string, WidgetMap>& GetLOKWeldWidgetsMap();
+    static void InsertWindowToMap(const std::string& nWindowId);
     void RememberWidget(const OString& id, weld::Widget* pWidget);
-    static weld::Widget* FindWeldWidgetsMap(sal_uInt64 nWindowId, const OString& rWidget);
+    static weld::Widget* FindWeldWidgetsMap(const std::string& nWindowId, const OString& rWidget);
+
+    std::string getMapIdFromWindowId() const;
 
     /// used for dialogs or popups
     JSInstanceBuilder(weld::Widget* pParent, const OUString& rUIRoot, const OUString& rUIFile,
@@ -281,8 +283,9 @@ public:
                                                     VclButtonsType eButtonType,
                                                     const OUString& rPrimaryMessage);
 
-    static void AddChildWidget(sal_uInt64 nWindowId, const OString& id, weld::Widget* pWidget);
-    static void RemoveWindowWidget(sal_uInt64 nWindowId);
+    static void AddChildWidget(const std::string& nWindowId, const OString& id,
+                               weld::Widget* pWidget);
+    static void RemoveWindowWidget(const std::string& nWindowId);
 
 private:
     const std::string& GetTypeOfJSON();
diff --git a/vcl/jsdialog/executor.cxx b/vcl/jsdialog/executor.cxx
index bddf849d503f..cddee247c531 100644
--- a/vcl/jsdialog/executor.cxx
+++ b/vcl/jsdialog/executor.cxx
@@ -34,7 +34,7 @@ StringMap jsonToStringMap(const char* pJSON)
     return aArgs;
 }
 
-void SendFullUpdate(sal_uInt64 nWindowId, const OString& rWidget)
+void SendFullUpdate(const std::string& nWindowId, const OString& rWidget)
 {
     weld::Widget* pWidget = JSInstanceBuilder::FindWeldWidgetsMap(nWindowId, rWidget);
 
@@ -45,7 +45,7 @@ void SendFullUpdate(sal_uInt64 nWindowId, const OString& rWidget)
     }
 }
 
-bool ExecuteAction(sal_uInt64 nWindowId, const OString& rWidget, StringMap& rData)
+bool ExecuteAction(const std::string& nWindowId, const OString& rWidget, StringMap& rData)
 {
     weld::Widget* pWidget = JSInstanceBuilder::FindWeldWidgetsMap(nWindowId, rWidget);
 
diff --git a/vcl/jsdialog/jsdialogbuilder.cxx b/vcl/jsdialog/jsdialogbuilder.cxx
index c74d0532a420..6bee71f4d71a 100644
--- a/vcl/jsdialog/jsdialogbuilder.cxx
+++ b/vcl/jsdialog/jsdialogbuilder.cxx
@@ -487,6 +487,14 @@ void JSDropTarget::fire_dragEnter(const css::datatransfer::dnd::DropTargetDragEn
     }
 }
 
+std::string JSInstanceBuilder::getMapIdFromWindowId() const
+{
+    if (m_sTypeOfJSON == "sidebar" || m_sTypeOfJSON == "notebookbar")
+        return std::to_string(m_nWindowId) + m_sTypeOfJSON;
+    else
+        return std::to_string(m_nWindowId);
+}
+
 // used for dialogs
 JSInstanceBuilder::JSInstanceBuilder(weld::Widget* pParent, const OUString& rUIRoot,
                                      const OUString& rUIFile, bool bPopup)
@@ -510,7 +518,7 @@ JSInstanceBuilder::JSInstanceBuilder(weld::Widget* pParent, const OUString& rUIR
         m_aParentDialog = pRoot->GetParent()->GetParentWithLOKNotifier();
         if (m_aParentDialog)
             m_nWindowId = m_aParentDialog->GetLOKWindowId();
-        InsertWindowToMap(m_nWindowId);
+        InsertWindowToMap(getMapIdFromWindowId());
     }
 
     initializeSender(GetNotifierWindow(), GetContentWindow(), GetTypeOfJSON());
@@ -547,7 +555,7 @@ JSInstanceBuilder::JSInstanceBuilder(weld::Widget* pParent, const OUString& rUIR
             m_aContentWindow = m_aContentWindow->GetParent();
     }
 
-    InsertWindowToMap(m_nWindowId);
+    InsertWindowToMap(getMapIdFromWindowId());
 
     initializeSender(GetNotifierWindow(), GetContentWindow(), GetTypeOfJSON());
 }
@@ -577,7 +585,7 @@ JSInstanceBuilder::JSInstanceBuilder(vcl::Window* pParent, const OUString& rUIRo
             m_nWindowId = nWindowId;
             m_bIsNotebookbar = true;
         }
-        InsertWindowToMap(m_nWindowId);
+        InsertWindowToMap(getMapIdFromWindowId());
     }
 
     initializeSender(GetNotifierWindow(), GetContentWindow(), GetTypeOfJSON());
@@ -602,7 +610,7 @@ JSInstanceBuilder::JSInstanceBuilder(vcl::Window* pParent, const OUString& rUIRo
         m_aParentDialog = pRoot->GetParent()->GetParentWithLOKNotifier();
         if (m_aParentDialog)
             m_nWindowId = m_aParentDialog->GetLOKWindowId();
-        InsertWindowToMap(m_nWindowId);
+        InsertWindowToMap(getMapIdFromWindowId());
     }
 
     initializeSender(GetNotifierWindow(), GetContentWindow(), GetTypeOfJSON());
@@ -655,11 +663,11 @@ JSInstanceBuilder::~JSInstanceBuilder()
 
     if (m_nWindowId && (m_bHasTopLevelDialog || m_bIsNotebookbar))
     {
-        GetLOKWeldWidgetsMap().erase(m_nWindowId);
+        GetLOKWeldWidgetsMap().erase(getMapIdFromWindowId());
     }
     else
     {
-        auto it = GetLOKWeldWidgetsMap().find(m_nWindowId);
+        auto it = GetLOKWeldWidgetsMap().find(getMapIdFromWindowId());
         if (it != GetLOKWeldWidgetsMap().end())
         {
             std::for_each(m_aRememberedWidgets.begin(), m_aRememberedWidgets.end(),
@@ -668,15 +676,16 @@ JSInstanceBuilder::~JSInstanceBuilder()
     }
 }
 
-std::map<sal_uInt64, WidgetMap>& JSInstanceBuilder::GetLOKWeldWidgetsMap()
+std::map<std::string, WidgetMap>& JSInstanceBuilder::GetLOKWeldWidgetsMap()
 {
     // Map to remember the LOKWindowId <-> weld widgets binding.
-    static std::map<sal_uInt64, WidgetMap> s_aLOKWeldBuildersMap;
+    static std::map<std::string, WidgetMap> s_aLOKWeldBuildersMap;
 
     return s_aLOKWeldBuildersMap;
 }
 
-weld::Widget* JSInstanceBuilder::FindWeldWidgetsMap(sal_uInt64 nWindowId, const OString& rWidget)
+weld::Widget* JSInstanceBuilder::FindWeldWidgetsMap(const std::string& nWindowId,
+                                                    const OString& rWidget)
 {
     const auto it = GetLOKWeldWidgetsMap().find(nWindowId);
 
@@ -690,17 +699,17 @@ weld::Widget* JSInstanceBuilder::FindWeldWidgetsMap(sal_uInt64 nWindowId, const
     return nullptr;
 }
 
-void JSInstanceBuilder::InsertWindowToMap(sal_uInt64 nWindowId)
+void JSInstanceBuilder::InsertWindowToMap(const std::string& nWindowId)
 {
     WidgetMap map;
     auto it = GetLOKWeldWidgetsMap().find(nWindowId);
     if (it == GetLOKWeldWidgetsMap().end())
-        GetLOKWeldWidgetsMap().insert(std::map<sal_uInt64, WidgetMap>::value_type(nWindowId, map));
+        GetLOKWeldWidgetsMap().insert(std::map<std::string, WidgetMap>::value_type(nWindowId, map));
 }
 
 void JSInstanceBuilder::RememberWidget(const OString& id, weld::Widget* pWidget)
 {
-    auto it = GetLOKWeldWidgetsMap().find(m_nWindowId);
+    auto it = GetLOKWeldWidgetsMap().find(getMapIdFromWindowId());
     if (it != GetLOKWeldWidgetsMap().end())
     {
         it->second.erase(id);
@@ -709,7 +718,7 @@ void JSInstanceBuilder::RememberWidget(const OString& id, weld::Widget* pWidget)
     }
 }
 
-void JSInstanceBuilder::AddChildWidget(sal_uInt64 nWindowId, const OString& id,
+void JSInstanceBuilder::AddChildWidget(const std::string& nWindowId, const OString& id,
                                        weld::Widget* pWidget)
 {
     auto it = GetLOKWeldWidgetsMap().find(nWindowId);
@@ -720,7 +729,7 @@ void JSInstanceBuilder::AddChildWidget(sal_uInt64 nWindowId, const OString& id,
     }
 }
 
-void JSInstanceBuilder::RemoveWindowWidget(sal_uInt64 nWindowId)
+void JSInstanceBuilder::RemoveWindowWidget(const std::string& nWindowId)
 {
     auto it = JSInstanceBuilder::GetLOKWeldWidgetsMap().find(nWindowId);
     if (it != JSInstanceBuilder::GetLOKWeldWidgetsMap().end())
@@ -754,7 +763,7 @@ std::unique_ptr<weld::Dialog> JSInstanceBuilder::weld_dialog(const OString& id)
         m_nWindowId = pDialog->GetLOKWindowId();
         pDialog->SetLOKTunnelingState(false);
 
-        InsertWindowToMap(m_nWindowId);
+        InsertWindowToMap(getMapIdFromWindowId());
 
         assert(!m_aOwnedToplevel && "only one toplevel per .ui allowed");
         m_aOwnedToplevel.set(pDialog);
@@ -782,7 +791,7 @@ std::unique_ptr<weld::MessageDialog> JSInstanceBuilder::weld_message_dialog(cons
         m_nWindowId = pMessageDialog->GetLOKWindowId();
         pMessageDialog->SetLOKTunnelingState(false);
 
-        InsertWindowToMap(m_nWindowId);
+        InsertWindowToMap(getMapIdFromWindowId());
 
         assert(!m_aOwnedToplevel && "only one toplevel per .ui allowed");
         m_aOwnedToplevel.set(pMessageDialog);
@@ -1030,7 +1039,7 @@ std::unique_ptr<weld::Popover> JSInstanceBuilder::weld_popover(const OString& id
             m_aParentDialog = pPopupRoot;
             m_aWindowToRelease = pPopupRoot;
             m_nWindowId = m_aParentDialog->GetLOKWindowId();
-            InsertWindowToMap(m_nWindowId);
+            InsertWindowToMap(getMapIdFromWindowId());
             initializeSender(GetNotifierWindow(), GetContentWindow(), GetTypeOfJSON());
         }
     }
@@ -1099,7 +1108,7 @@ weld::MessageDialog* JSInstanceBuilder::CreateMessageDialog(weld::Widget* pParen
     }
 
     xMessageDialog->SetLOKTunnelingState(false);
-    InsertWindowToMap(xMessageDialog->GetLOKWindowId());
+    InsertWindowToMap(std::to_string(xMessageDialog->GetLOKWindowId()));
     return new JSMessageDialog(xMessageDialog, nullptr, true);
 }
 
@@ -1308,7 +1317,7 @@ JSMessageDialog::JSMessageDialog(::MessageDialog* pDialog, SalInstanceBuilder* p
             = dynamic_cast<::OKButton*>(m_xMessageDialog->get_widget_for_response(RET_OK)))
         {
             m_pOK.reset(new JSButton(m_pSender, pOKBtn, nullptr, false));
-            JSInstanceBuilder::AddChildWidget(m_xMessageDialog->GetLOKWindowId(),
+            JSInstanceBuilder::AddChildWidget(std::to_string(m_xMessageDialog->GetLOKWindowId()),
                                               pOKBtn->get_id().toUtf8(), m_pOK.get());
             m_pOK->connect_clicked(LINK(this, JSMessageDialog, OKHdl));
         }
@@ -1317,7 +1326,7 @@ JSMessageDialog::JSMessageDialog(::MessageDialog* pDialog, SalInstanceBuilder* p
             = dynamic_cast<::CancelButton*>(m_xMessageDialog->get_widget_for_response(RET_CANCEL)))
         {
             m_pCancel.reset(new JSButton(m_pSender, pCancelBtn, nullptr, false));
-            JSInstanceBuilder::AddChildWidget(m_xMessageDialog->GetLOKWindowId(),
+            JSInstanceBuilder::AddChildWidget(std::to_string(m_xMessageDialog->GetLOKWindowId()),
                                               pCancelBtn->get_id().toUtf8(), m_pCancel.get());
             m_pCancel->connect_clicked(LINK(this, JSMessageDialog, CancelHdl));
         }
@@ -1327,7 +1336,7 @@ JSMessageDialog::JSMessageDialog(::MessageDialog* pDialog, SalInstanceBuilder* p
 JSMessageDialog::~JSMessageDialog()
 {
     if (m_pOK || m_pCancel)
-        JSInstanceBuilder::RemoveWindowWidget(m_xMessageDialog->GetLOKWindowId());
+        JSInstanceBuilder::RemoveWindowWidget(std::to_string(m_xMessageDialog->GetLOKWindowId()));
 }
 
 IMPL_LINK_NOARG(JSMessageDialog, OKHdl, weld::Button&, void) { response(RET_OK); }