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)