desktop/source/lib/init.cxx                  |   54 ++++-----------------------
 include/sfx2/sidebar/Sidebar.hxx             |    2 +
 sd/qa/unit/tiledrendering/tiledrendering.cxx |   45 ++++++++++++++++++++++
 sfx2/source/sidebar/Sidebar.cxx              |   46 +++++++++++++++++++++++
 sfx2/source/sidebar/SidebarController.cxx    |   10 ++++-
 5 files changed, 111 insertions(+), 46 deletions(-)

New commits:
commit ccc5f1cdcb11f6b6c5e54f1e4dd00b9c6771d077
Author:     Miklos Vajna <vmik...@collabora.com>
AuthorDate: Wed Mar 20 12:08:42 2024 +0100
Commit:     Caolán McNamara <caolan.mcnam...@collabora.com>
CommitDate: Thu Mar 21 15:54:25 2024 +0100

    cool#8278 sfx2 lok: fix unexpected non-json sidebar status update
    
    Open an Impress document via LOK, open the slide layout sidebar, click
    the toggle icon so it gets closed: the toggle icon will signal that the
    sidebar is open, when it's closed already.
    
    This is a regression from commit
    aaf6ce108e91b1504befe19afcee471e3316ae7a (cool#7492 sfx2 lok: set
    language/locale on async sidebar update, 2024-01-11), previously we
    always emitted LOK_CALLBACK_STATE_CHANGED callbacks with plain text
    payloads for the sidebar, where the locale with implicit (with all its
    issues), but the above scenario worked fine.
    
    Fix the problem by making SidebarController::disposeDecks() consistent
    with SwitchToDeck(), so now we always emit JSON payloads for the sidebar
    deck changes.
    
    An alternative would be to improve the code around extractUnoCommand()
    in online.git to handle a mix of plain text and JSON payloads, but the
    plain text payload is tricky to extend, so using more JSON payloads
    sounds like a better fix.
    
    (cherry picked from commit 55feb670ca28e0a48ac82a65b5559598704d993e)
    
    Conflicts:
            desktop/source/lib/init.cxx
            sd/qa/unit/tiledrendering/tiledrendering.cxx
            sfx2/source/sidebar/SidebarController.cxx
    
    Change-Id: I5b75c2987c230c6720181a1e95ae579727943235
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/165101
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
    Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>

diff --git a/desktop/source/lib/init.cxx b/desktop/source/lib/init.cxx
index c3183f0ae093..9288a67c2181 100644
--- a/desktop/source/lib/init.cxx
+++ b/desktop/source/lib/init.cxx
@@ -152,6 +152,7 @@
 #include <sfx2/DocumentSigner.hxx>
 #include <sfx2/sidebar/SidebarDockingWindow.hxx>
 #include <sfx2/sidebar/SidebarController.hxx>
+#include <sfx2/sidebar/Sidebar.hxx>
 #include <svl/numformat.hxx>
 #include <svx/dialmgr.hxx>
 #include <svx/strings.hrc>
@@ -1009,49 +1010,6 @@ void ExecuteOrientationChange()
         mxUndoManager->leaveUndoContext();
 }
 
-void setupSidebar(std::u16string_view sidebarDeckId = u"")
-{
-    SfxViewShell* pViewShell = SfxViewShell::Current();
-    SfxViewFrame* pViewFrame = pViewShell ? pViewShell->GetViewFrame() : 
nullptr;
-    if (pViewFrame)
-    {
-        if (!pViewFrame->GetChildWindow(SID_SIDEBAR))
-            pViewFrame->SetChildWindow(SID_SIDEBAR, false /* create it */, 
true /* focus */);
-
-        pViewFrame->ShowChildWindow(SID_SIDEBAR, true);
-
-        // Force synchronous population of panels
-        SfxChildWindow *pChild = pViewFrame->GetChildWindow(SID_SIDEBAR);
-        if (!pChild)
-            return;
-
-        auto pDockingWin = dynamic_cast<sfx2::sidebar::SidebarDockingWindow 
*>(pChild->GetWindow());
-        if (!pDockingWin)
-            return;
-
-        pViewFrame->ShowChildWindow( SID_SIDEBAR );
-
-        const rtl::Reference<sfx2::sidebar::SidebarController>& xController
-            = pDockingWin->GetOrCreateSidebarController();
-
-        xController->FadeIn();
-        xController->RequestOpenDeck();
-
-        if (!sidebarDeckId.empty())
-        {
-            xController->SwitchToDeck(sidebarDeckId);
-        }
-        else
-        {
-            xController->SwitchToDefaultDeck();
-        }
-
-        pDockingWin->SyncUpdate();
-    }
-    else
-        SetLastExceptionMsg("No view shell or sidebar");
-}
-
 void hideSidebar()
 {
     SfxViewShell* pViewShell = SfxViewShell::Current();
@@ -5300,12 +5258,18 @@ static void doc_postUnoCommand(LibreOfficeKitDocument* 
pThis, const char* pComma
     }
     else if (gImpl && aCommand == ".uno:LOKSidebarWriterPage")
     {
-        setupSidebar(u"WriterPageDeck");
+        if (!sfx2::sidebar::Sidebar::Setup(u"WriterPageDeck"))
+        {
+            SetLastExceptionMsg("failed to set up sidebar");
+        }
         return;
     }
     else if (gImpl && aCommand == ".uno:SidebarShow")
     {
-        setupSidebar();
+        if (!sfx2::sidebar::Sidebar::Setup(u""))
+        {
+            SetLastExceptionMsg("failed to set up sidebar");
+        }
         return;
     }
     else if (gImpl && aCommand == ".uno:SidebarHide")
diff --git a/include/sfx2/sidebar/Sidebar.hxx b/include/sfx2/sidebar/Sidebar.hxx
index 2520a80a4be4..c3ec9b8d40ac 100644
--- a/include/sfx2/sidebar/Sidebar.hxx
+++ b/include/sfx2/sidebar/Sidebar.hxx
@@ -60,6 +60,8 @@ public:
     static bool IsPanelVisible(
         std::u16string_view rsPanelId,
         const css::uno::Reference<css::frame::XFrame>& rxFrame);
+
+    static bool Setup(std::u16string_view sidebarDeckId = u"");
 };
 
 } // end of namespace sfx2::sidebar
diff --git a/sd/qa/unit/tiledrendering/tiledrendering.cxx 
b/sd/qa/unit/tiledrendering/tiledrendering.cxx
index fd990b54c757..236185d73c73 100644
--- a/sd/qa/unit/tiledrendering/tiledrendering.cxx
+++ b/sd/qa/unit/tiledrendering/tiledrendering.cxx
@@ -54,6 +54,7 @@
 #include <vcl/BitmapReadAccess.hxx>
 #include <vcl/virdev.hxx>
 #include <o3tl/string_view.hxx>
+#include <sfx2/sidebar/Sidebar.hxx>
 
 #include <chrono>
 #include <cstdlib>
@@ -132,6 +133,7 @@ public:
     void testDeleteTable();
     void testPasteUndo();
     void testShapeEditInMultipleViews();
+    void testSidebarHide();
     void testGetViewRenderState();
 
     CPPUNIT_TEST_SUITE(SdTiledRenderingTest);
@@ -193,6 +195,7 @@ public:
     CPPUNIT_TEST(testDeleteTable);
     CPPUNIT_TEST(testPasteUndo);
     CPPUNIT_TEST(testShapeEditInMultipleViews);
+    CPPUNIT_TEST(testSidebarHide);
     CPPUNIT_TEST(testGetViewRenderState);
     CPPUNIT_TEST_SUITE_END();
 
@@ -908,6 +911,7 @@ public:
     bool m_bViewSelectionSet;
     boost::property_tree::ptree m_aCommentCallbackResult;
     OString m_ShapeSelection;
+    std::map<std::string, boost::property_tree::ptree> m_aStateChanges;
     TestLokCallbackWrapper m_callbackWrapper;
 
     ViewCallback()
@@ -1020,6 +1024,27 @@ public:
             m_aCommentCallbackResult = 
m_aCommentCallbackResult.get_child("comment");
         }
         break;
+        case LOK_CALLBACK_STATE_CHANGED:
+        {
+            OString aPayload(pPayload);
+            if (!aPayload.startsWith("{"))
+            {
+                break;
+            }
+
+            std::stringstream aStream(pPayload);
+            boost::property_tree::ptree aTree;
+            boost::property_tree::read_json(aStream, aTree);
+            auto it = aTree.find("commandName");
+            if (it == aTree.not_found())
+            {
+                break;
+            }
+
+            std::string aCommandName = it->second.get_value<std::string>();
+            m_aStateChanges[aCommandName] = aTree;
+        }
+        break;
         }
     }
 };
@@ -3096,6 +3121,26 @@ void SdTiledRenderingTest::testShapeEditInMultipleViews()
     }
 }
 
+void SdTiledRenderingTest::testSidebarHide()
+{
+    // Given an impress document, with a visible sidebar:
+    createDoc("dummy.odp");
+    ViewCallback aView;
+    sfx2::sidebar::Sidebar::Setup(u"");
+    Scheduler::ProcessEventsToIdle();
+    aView.m_aStateChanges.clear();
+
+    // When hiding the slide layout deck:
+    dispatchCommand(mxComponent, ".uno:ModifyPage", {});
+    Scheduler::ProcessEventsToIdle();
+
+    // Then make sure we get a state change for this, in JSON format:
+    auto it = aView.m_aStateChanges.find(".uno:ModifyPage");
+    // Without the accompanying fix in place, this test would have failed, we 
got the state change
+    // in plain text, which was inconsistent (show was JSON, hide was plain 
text).
+    CPPUNIT_ASSERT(it != aView.m_aStateChanges.end());
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(SdTiledRenderingTest);
 
 CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/sfx2/source/sidebar/Sidebar.cxx b/sfx2/source/sidebar/Sidebar.cxx
index ca48542d5403..e7bc34e2921b 100644
--- a/sfx2/source/sidebar/Sidebar.cxx
+++ b/sfx2/source/sidebar/Sidebar.cxx
@@ -20,11 +20,13 @@
 #include <sfx2/sidebar/Sidebar.hxx>
 #include <sfx2/sidebar/SidebarController.hxx>
 #include <sfx2/sidebar/ResourceManager.hxx>
+#include <sfx2/sidebar/SidebarDockingWindow.hxx>
 #include <sidebar/PanelDescriptor.hxx>
 #include <sidebar/Tools.hxx>
 #include <sfx2/sidebar/FocusManager.hxx>
 #include <sfx2/childwin.hxx>
 #include <sfx2/sfxsids.hrc>
+#include <sfx2/viewsh.hxx>
 #include <com/sun/star/frame/XDispatch.hpp>
 
 using namespace css;
@@ -123,6 +125,50 @@ bool Sidebar::IsPanelVisible(
     return pController->IsDeckVisible(xPanelDescriptor->msDeckId);
 }
 
+bool Sidebar::Setup(std::u16string_view sidebarDeckId)
+{
+    SfxViewShell* pViewShell = SfxViewShell::Current();
+    SfxViewFrame* pViewFrame = pViewShell ? pViewShell->GetViewFrame() : 
nullptr;
+    if (pViewFrame)
+    {
+        if (!pViewFrame->GetChildWindow(SID_SIDEBAR))
+            pViewFrame->SetChildWindow(SID_SIDEBAR, false /* create it */, 
true /* focus */);
+
+        pViewFrame->ShowChildWindow(SID_SIDEBAR, true);
+
+        // Force synchronous population of panels
+        SfxChildWindow *pChild = pViewFrame->GetChildWindow(SID_SIDEBAR);
+        if (!pChild)
+            return false;
+
+        auto pDockingWin = dynamic_cast<sfx2::sidebar::SidebarDockingWindow 
*>(pChild->GetWindow());
+        if (!pDockingWin)
+            return false;
+
+        pViewFrame->ShowChildWindow( SID_SIDEBAR );
+
+        const rtl::Reference<sfx2::sidebar::SidebarController>& xController
+            = pDockingWin->GetOrCreateSidebarController();
+
+        xController->FadeIn();
+        xController->RequestOpenDeck();
+
+        if (!sidebarDeckId.empty())
+        {
+            xController->SwitchToDeck(sidebarDeckId);
+        }
+        else
+        {
+            xController->SwitchToDefaultDeck();
+        }
+
+        pDockingWin->SyncUpdate();
+        return true;
+    }
+    else
+        return false;
+}
+
 } // end of namespace sfx2::sidebar
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sfx2/source/sidebar/SidebarController.cxx 
b/sfx2/source/sidebar/SidebarController.cxx
index 611a84b92ab9..82dee4895f03 100644
--- a/sfx2/source/sidebar/SidebarController.cxx
+++ b/sfx2/source/sidebar/SidebarController.cxx
@@ -229,8 +229,16 @@ void SidebarController::disposeDecks()
         {
             const std::string hide = UnoNameFromDeckId(msCurrentDeckId, 
GetCurrentContext());
             if (!hide.empty())
+            {
+                // Be consistent with SwitchToDeck(), so both places emit JSON.
+                boost::property_tree::ptree aTree;
+                aTree.put("commandName", hide);
+                aTree.put("state", "false");
+                std::stringstream aStream;
+                boost::property_tree::write_json(aStream, aTree);
                 
pViewShell->libreOfficeKitViewCallback(LOK_CALLBACK_STATE_CHANGED,
-                                                       (hide + 
"=false").c_str());
+                                                       aStream.str().c_str());
+            }
         }
 
         if (mpParentWindow)

Reply via email to