sw: make shift-doubleclick on graphics open the graphic dialog

Double-clicking on images without modifier keys or with ctrl triggered
the graphic dialog. However, shift-doubleclick did not.

One drawback of this behavior is that extensions can catch the
.uno:GraphicDialog command for plain or ctrl double-click, but they
can't customize the behavior for shift double-click. Another problem was
that if you had a text selection and you shift-clicked (single click) on
an image, then no image selection was created. (While this works in
Word.)

Fix the problem by extending the code handling mouse button events in
Writer in case the button type is left, the only modifier is shift and
we get one or two clicks:

- if the position points to an object that should be selected, then
shift-click now selects that object

- if the user shift-double-clicks on an image, then we open the graphic
dialog

This improves consistency, as double-click with no/ctrl/shift modifiers
now all dispatch .uno:GraphicDialog. Do the double-click tweak only for
graphics, other shape types can be added in the future if needed.

Change-Id: I2edc70a399a9d2662b476a1bbc4c5f087ce8e59c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/137696
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins

4 files changed, 292 insertions, 0 deletions

diff --git a/sw/CppunitTest_sw_uibase_docvw.mk b/sw/CppunitTest_sw_uibase_docvw.mk
new file mode 100644
index 000000000000..2938c8e3a6dd
--- /dev/null
+++ b/sw/CppunitTest_sw_uibase_docvw.mk
@@ -0,0 +1,74 @@
+# -*- Mode: makefile-gmake; tab-width: 4; indent-tabs-mode: t -*-
+#*************************************************************************
+#
+# This file is part of the LibreOffice project.
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+#*************************************************************************
+
+$(eval $(call gb_CppunitTest_CppunitTest,sw_uibase_docvw))
+
+$(eval $(call gb_CppunitTest_use_common_precompiled_header,sw_uibase_docvw))
+
+$(eval $(call gb_CppunitTest_add_exception_objects,sw_uibase_docvw, \
+    sw/qa/uibase/docvw/docvw \
+))
+
+$(eval $(call gb_CppunitTest_use_libraries,sw_uibase_docvw, \
+    comphelper \
+    cppu \
+    cppuhelper \
+    editeng \
+    sal \
+    sfx \
+    svl \
+    svx \
+    svxcore \
+    sw \
+    swqahelper \
+    test \
+    unotest \
+    utl \
+    vcl \
+))
+
+$(eval $(call gb_CppunitTest_use_externals,sw_uibase_docvw,\
+    boost_headers \
+    libxml2 \
+))
+
+$(eval $(call gb_CppunitTest_set_include,sw_uibase_docvw,\
+    -I$(SRCDIR)/sw/inc \
+    -I$(SRCDIR)/sw/source/core/inc \
+    -I$(SRCDIR)/sw/source/uibase/inc \
+    -I$(SRCDIR)/sw/qa/inc \
+    $$(INCLUDE) \
+))
+
+$(eval $(call gb_CppunitTest_use_api,sw_uibase_docvw,\
+	udkapi \
+	offapi \
+	oovbaapi \
+))
+
+$(eval $(call gb_CppunitTest_use_ure,sw_uibase_docvw))
+$(eval $(call gb_CppunitTest_use_vcl,sw_uibase_docvw))
+
+$(eval $(call gb_CppunitTest_use_rdb,sw_uibase_docvw,services))
+
+$(eval $(call gb_CppunitTest_use_custom_headers,sw_uibase_docvw,\
+    officecfg/registry \
+))
+
+$(eval $(call gb_CppunitTest_use_configuration,sw_uibase_docvw))
+
+$(eval $(call gb_CppunitTest_use_uiconfigs,sw_uibase_docvw, \
+    modules/swriter \
+))
+
+$(eval $(call gb_CppunitTest_use_more_fonts,sw_uibase_docvw))
+
+# vim: set noet sw=4 ts=4:
diff --git a/sw/Module_sw.mk b/sw/Module_sw.mk
index 318f8581d7df..146b6c7bf138 100644
--- a/sw/Module_sw.mk
+++ b/sw/Module_sw.mk
@@ -128,6 +128,7 @@ $(eval $(call gb_Module_add_slowcheck_targets,sw,\
     CppunitTest_sw_core_docnode \
     CppunitTest_sw_uibase_shells \
     CppunitTest_sw_uibase_dochdl \
+    CppunitTest_sw_uibase_docvw \
     CppunitTest_sw_uibase_frmdlg \
     CppunitTest_sw_uibase_uno \
     CppunitTest_sw_uibase_wrtsh \
diff --git a/sw/qa/uibase/docvw/docvw.cxx b/sw/qa/uibase/docvw/docvw.cxx
new file mode 100644
index 000000000000..3ec8027c8468
--- /dev/null
+++ b/sw/qa/uibase/docvw/docvw.cxx
@@ -0,0 +1,190 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <swmodeltestbase.hxx>
+
+#include <com/sun/star/text/XTextDocument.hpp>
+
+#include <vcl/event.hxx>
+
+#include <docsh.hxx>
+#include <edtwin.hxx>
+#include <flyfrm.hxx>
+#include <frameformats.hxx>
+#include <view.hxx>
+#include <wrtsh.hxx>
+
+namespace
+{
+/// Covers sw/source/uibase/docvw/ fixes.
+class Test : public SwModelTestBase
+{
+};
+}
+
+CPPUNIT_TEST_FIXTURE(Test, testShiftClickOnImage)
+{
+    // Given a document with a fly frame:
+    SwDoc* pDoc = createSwDoc();
+    SwWrtShell* pWrtShell = pDoc->GetDocShell()->GetWrtShell();
+    uno::Reference<lang::XMultiServiceFactory> xMSF(mxComponent, uno::UNO_QUERY);
+    uno::Reference<text::XTextDocument> xTextDocument(mxComponent, uno::UNO_QUERY);
+    uno::Reference<text::XText> xText = xTextDocument->getText();
+    uno::Reference<text::XTextCursor> xCursor = xText->createTextCursor();
+    uno::Reference<beans::XPropertySet> xTextGraphic(
+        xMSF->createInstance("com.sun.star.text.TextGraphicObject"), uno::UNO_QUERY);
+    xTextGraphic->setPropertyValue("AnchorType",
+                                   uno::Any(text::TextContentAnchorType_AS_CHARACTER));
+    xTextGraphic->setPropertyValue("Size", uno::Any(awt::Size(5000, 5000)));
+    uno::Reference<text::XTextContent> xTextContent(xTextGraphic, uno::UNO_QUERY);
+    xText->insertTextContent(xCursor, xTextContent, false);
+    pWrtShell->SttEndDoc(/*bStt=*/false);
+
+    // When shift-clicking on that fly frame:
+    SwFrameFormats& rSpzFormats = *pDoc->GetSpzFrameFormats();
+    auto pFrameFormat = dynamic_cast<SwFlyFrameFormat*>(rSpzFormats[0]);
+    CPPUNIT_ASSERT(pFrameFormat);
+    SwFlyFrame* pFlyFrame = pFrameFormat->GetFrame();
+    vcl::Window& rEditWin = pDoc->GetDocShell()->GetView()->GetEditWin();
+    Point aFlyCenter = rEditWin.LogicToPixel(pFlyFrame->getFrameArea().Center());
+    MouseEvent aClickEvent(aFlyCenter, 1, MouseEventModifiers::SIMPLECLICK, MOUSE_LEFT, KEY_SHIFT);
+    rEditWin.MouseButtonDown(aClickEvent);
+    rEditWin.MouseButtonUp(aClickEvent);
+
+    // Then make sure that the fly frame is selected:
+    SelectionType eType = pWrtShell->GetSelectionType();
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected: 2 (SelectionType::Graphic)
+    // - Actual  : 1 (SelectionType::Text)
+    // i.e. the fly frame was not selected, while a plain click or ctrl-click selected it.
+    CPPUNIT_ASSERT_EQUAL(SelectionType::Graphic, eType);
+}
+
+namespace
+{
+/// Interception implementation that catches the graphic dialog.
+class GraphicDialogInterceptor : public cppu::WeakImplHelper<frame::XDispatchProviderInterceptor>
+{
+    uno::Reference<frame::XDispatchProvider> m_xMaster;
+    uno::Reference<frame::XDispatchProvider> m_xSlave;
+    int m_nGraphicDialogs = 0;
+
+public:
+    // XDispatchProviderInterceptor
+    uno::Reference<frame::XDispatchProvider> SAL_CALL getMasterDispatchProvider() override;
+    uno::Reference<frame::XDispatchProvider> SAL_CALL getSlaveDispatchProvider() override;
+    void SAL_CALL setMasterDispatchProvider(
+        const uno::Reference<frame::XDispatchProvider>& xNewSupplier) override;
+    void SAL_CALL
+    setSlaveDispatchProvider(const uno::Reference<frame::XDispatchProvider>& xNewSupplier) override;
+
+    // XDispatchProvider
+    uno::Reference<frame::XDispatch> SAL_CALL queryDispatch(const util::URL& rURL,
+                                                            const OUString& rTargetFrameName,
+                                                            sal_Int32 SearchFlags) override;
+    uno::Sequence<uno::Reference<frame::XDispatch>> SAL_CALL
+    queryDispatches(const uno::Sequence<frame::DispatchDescriptor>& rRequests) override;
+
+    int GetGraphicDialogs() const;
+};
+}
+
+uno::Reference<frame::XDispatchProvider> GraphicDialogInterceptor::getMasterDispatchProvider()
+{
+    return m_xMaster;
+}
+
+uno::Reference<frame::XDispatchProvider> GraphicDialogInterceptor::getSlaveDispatchProvider()
+{
+    return m_xSlave;
+}
+
+void GraphicDialogInterceptor::setMasterDispatchProvider(
+    const uno::Reference<frame::XDispatchProvider>& xNewSupplier)
+{
+    m_xMaster = xNewSupplier;
+}
+
+void GraphicDialogInterceptor::setSlaveDispatchProvider(
+    const uno::Reference<frame::XDispatchProvider>& xNewSupplier)
+{
+    m_xSlave = xNewSupplier;
+}
+
+uno::Reference<frame::XDispatch>
+GraphicDialogInterceptor::queryDispatch(const util::URL& rURL, const OUString& rTargetFrameName,
+                                        sal_Int32 nSearchFlags)
+{
+    if (rURL.Complete == ".uno:GraphicDialog")
+    {
+        ++m_nGraphicDialogs;
+    }
+
+    return m_xSlave->queryDispatch(rURL, rTargetFrameName, nSearchFlags);
+}
+
+uno::Sequence<uno::Reference<frame::XDispatch>> GraphicDialogInterceptor::queryDispatches(
+    const uno::Sequence<frame::DispatchDescriptor>& /*rRequests*/)
+{
+    return {};
+}
+
+int GraphicDialogInterceptor::GetGraphicDialogs() const { return m_nGraphicDialogs; }
+
+CPPUNIT_TEST_FIXTURE(Test, testShiftDoubleClickOnImage)
+{
+    // Given a document with a fly frame, and an interceptor to catch the graphic dialog:
+    SwDoc* pDoc = createSwDoc();
+    SwWrtShell* pWrtShell = pDoc->GetDocShell()->GetWrtShell();
+    uno::Reference<lang::XMultiServiceFactory> xMSF(mxComponent, uno::UNO_QUERY);
+    uno::Reference<text::XTextDocument> xTextDocument(mxComponent, uno::UNO_QUERY);
+    uno::Reference<text::XText> xText = xTextDocument->getText();
+    uno::Reference<text::XTextCursor> xCursor = xText->createTextCursor();
+    uno::Reference<beans::XPropertySet> xTextGraphic(
+        xMSF->createInstance("com.sun.star.text.TextGraphicObject"), uno::UNO_QUERY);
+    xTextGraphic->setPropertyValue("AnchorType",
+                                   uno::Any(text::TextContentAnchorType_AS_CHARACTER));
+    xTextGraphic->setPropertyValue("Size", uno::Any(awt::Size(5000, 5000)));
+    uno::Reference<text::XTextContent> xTextContent(xTextGraphic, uno::UNO_QUERY);
+    xText->insertTextContent(xCursor, xTextContent, false);
+    pWrtShell->SttEndDoc(/*bStt=*/false);
+    uno::Reference<frame::XModel> xModel(mxComponent, uno::UNO_QUERY);
+    uno::Reference<frame::XDispatchProviderInterception> xRegistration(
+        xModel->getCurrentController()->getFrame(), uno::UNO_QUERY);
+    rtl::Reference pInterceptor(new GraphicDialogInterceptor);
+    xRegistration->registerDispatchProviderInterceptor(pInterceptor);
+
+    // When shift-double-clicking on that fly frame:
+    SwFrameFormats& rSpzFormats = *pDoc->GetSpzFrameFormats();
+    auto pFrameFormat = dynamic_cast<SwFlyFrameFormat*>(rSpzFormats[0]);
+    CPPUNIT_ASSERT(pFrameFormat);
+    SwFlyFrame* pFlyFrame = pFrameFormat->GetFrame();
+    vcl::Window& rEditWin = pDoc->GetDocShell()->GetView()->GetEditWin();
+    Point aFlyCenter = rEditWin.LogicToPixel(pFlyFrame->getFrameArea().Center());
+    MouseEvent aClickEvent(aFlyCenter, 1, MouseEventModifiers::SIMPLECLICK, MOUSE_LEFT, KEY_SHIFT);
+    rEditWin.MouseButtonDown(aClickEvent);
+    rEditWin.MouseButtonUp(aClickEvent);
+    aClickEvent
+        = MouseEvent(aFlyCenter, 2, MouseEventModifiers::SIMPLECLICK, MOUSE_LEFT, KEY_SHIFT);
+    rEditWin.MouseButtonDown(aClickEvent);
+    rEditWin.MouseButtonUp(aClickEvent);
+
+    // Then make sure that the fly frame's dialog is dispatched:
+    int nGraphicDialogs = pInterceptor->GetGraphicDialogs();
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected greater than: 0 (2)
+    // - Actual  : 0
+    // i.e. the fly frame's dialog was not dispatched, while a plain click or ctrl-click dispatched
+    // it.
+    CPPUNIT_ASSERT_GREATER(0, nGraphicDialogs);
+}
+
+CPPUNIT_PLUGIN_IMPLEMENT();
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/uibase/docvw/edtwin.cxx b/sw/source/uibase/docvw/edtwin.cxx
index 79f980360c56..863b5ac6c65e 100644
--- a/sw/source/uibase/docvw/edtwin.cxx
+++ b/sw/source/uibase/docvw/edtwin.cxx
@@ -3665,6 +3665,19 @@ void SwEditWin::MouseButtonDown(const MouseEvent& _rMEvt)
                     break;
                     case KEY_SHIFT:
                     {
+                        if (nNumberOfClicks == 2)
+                        {
+                            // Left mouse button, shift, double-click: see if we have a graphic and
+                            // dispatch its dialog in this case.
+                            if (rSh.GetSelectionType() == SelectionType::Graphic)
+                            {
+                                GetView().GetViewFrame()->GetBindings().Execute(
+                                    FN_FORMAT_GRAFIC_DLG, nullptr,
+                                    SfxCallMode::RECORD | SfxCallMode::SLOT);
+                                return;
+                            }
+                        }
+
                         if ( !m_bInsDraw && IsDrawObjSelectable( rSh, aDocPos ) )
                         {
                             m_rView.NoRotate();
@@ -3694,6 +3707,20 @@ void SwEditWin::MouseButtonDown(const MouseEvent& _rMEvt)
                         }
                         else
                         {
+                            bool bShould = rSh.ShouldObjectBeSelected(aDocPos);
+                            if (bShould)
+                            {
+                                // Left mouse button, shift, non-double-click, not a draw object and
+                                // have an object to select: select it.
+                                rSh.HideCursor();
+                                bool bSelObj = rSh.SelectObj(aDocPos);
+                                if (bSelObj)
+                                {
+                                    rSh.EnterSelFrameMode(&aDocPos);
+                                }
+                                return;
+                            }
+
                             if ( rSh.IsSelFrameMode() &&
                                  rSh.IsInsideSelectedObj( aDocPos ) )
                             {