sw, createTextRangeByPixelPosition(): fix crash when the position is an image

Using createTextRangeByPixelPosition() with a pixel position that leads
to a graphic node resulted in a crash.

The direct reason for this is that the makeMark() call in
SwXTextRange::SetPositions() returns nullptr in case rPaM points to a
graphic node, but later we dereference that result unconditionally. This
also uncovers that the XTextRange returned by
createTextRangeByPixelPosition() is meant to point to a text node, but a
pixel position may be closest to a graphic node.

Fix the problem by explicitly checking for graphic nodes; and try to
look up the anchor position of such graphics, which will be definitely a
text node.

In practice this will mean that in case the image's anchor type is
as-char, then we'll return a cursor position which will be on the left
hand side of the image.

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

3 files changed, 81 insertions, 1 deletions

diff --git a/offapi/com/sun/star/text/XTextViewTextRangeSupplier.idl b/offapi/com/sun/star/text/XTextViewTextRangeSupplier.idl
index 56ea30efc070..faa85578c458 100644
--- a/offapi/com/sun/star/text/XTextViewTextRangeSupplier.idl
+++ b/offapi/com/sun/star/text/XTextViewTextRangeSupplier.idl
@@ -25,7 +25,12 @@ module com {  module sun {  module star {  module text {
  */
 interface XTextViewTextRangeSupplier: com::sun::star::uno::XInterface
 {
-    /** @returns
+    /** creates the text range of the document model position at a view-dependent pixel position.
+
+        Note that in case the model position is a graphic, then the model position of its anchor is
+        returned.
+
+        @returns
                 the text range of the document position.
      */
     com::sun::star::text::XTextRange createTextRangeByPixelPosition([in] com::sun::star::awt::Point PixelPosition);
diff --git a/sw/qa/uibase/uno/uno.cxx b/sw/qa/uibase/uno/uno.cxx
index a1484b1ce629..3364ef8b0cd2 100644
--- a/sw/qa/uibase/uno/uno.cxx
+++ b/sw/qa/uibase/uno/uno.cxx
@@ -12,6 +12,7 @@
 #include <com/sun/star/frame/XModel2.hpp>
 #include <com/sun/star/text/XTextViewTextRangeSupplier.hpp>
 #include <com/sun/star/util/XCloseable.hpp>
+#include <com/sun/star/text/XTextDocument.hpp>
 
 #include <vcl/scheduler.hxx>
 
@@ -20,6 +21,11 @@
 #include <unotextrange.hxx>
 #include <view.hxx>
 #include <wrtsh.hxx>
+#include <rootfrm.hxx>
+#include <sortedobjs.hxx>
+#include <anchoredobject.hxx>
+#include <frameformats.hxx>
+#include <fmtanchr.hxx>
 
 /// Covers sw/source/uibase/uno/ fixes.
 class SwUibaseUnoTest : public SwModelTestBase
@@ -94,6 +100,57 @@ CPPUNIT_TEST_FIXTURE(SwUibaseUnoTest, testCreateTextRangeByPixelPosition)
     CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(1), nActual);
 }
 
+CPPUNIT_TEST_FIXTURE(SwUibaseUnoTest, testCreateTextRangeByPixelPositionGraphic)
+{
+    // Given a document with an as-char image and the center of that image in pixels:
+    createSwDoc();
+    uno::Reference<lang::XMultiServiceFactory> xFactory(mxComponent, uno::UNO_QUERY);
+    uno::Reference<beans::XPropertySet> xTextGraphic(
+        xFactory->createInstance("com.sun.star.text.TextGraphicObject"), uno::UNO_QUERY);
+    xTextGraphic->setPropertyValue("AnchorType",
+                                   uno::Any(text::TextContentAnchorType_AS_CHARACTER));
+    xTextGraphic->setPropertyValue("Width", uno::Any(static_cast<sal_Int32>(10000)));
+    xTextGraphic->setPropertyValue("Height", uno::Any(static_cast<sal_Int32>(10000)));
+    uno::Reference<text::XTextDocument> xTextDocument(mxComponent, uno::UNO_QUERY);
+    uno::Reference<text::XText> xBodyText = xTextDocument->getText();
+    uno::Reference<text::XTextCursor> xCursor(xBodyText->createTextCursor());
+    uno::Reference<text::XTextContent> xTextContent(xTextGraphic, uno::UNO_QUERY);
+    xBodyText->insertTextContent(xCursor, xTextContent, false);
+    SwDoc* pDoc = getSwDoc();
+    SwDocShell* pDocShell = pDoc->GetDocShell();
+    SwWrtShell* pWrtShell = pDocShell->GetWrtShell();
+    SwRootFrame* pLayout = pWrtShell->GetLayout();
+    SwFrame* pPage = pLayout->GetLower();
+    SwFrame* pBody = pPage->GetLower();
+    SwFrame* pText = pBody->GetLower();
+    SwSortedObjs& rDrawObjs = *pText->GetDrawObjs();
+    SwAnchoredObject* pAnchored = rDrawObjs[0];
+    Point aLogic = pAnchored->GetObjRect().Center();
+    SwView* pView = pDocShell->GetView();
+    SwEditWin& rEditWin = pView->GetEditWin();
+    Point aPixel = rEditWin.LogicToPixel(aLogic);
+
+    // When converting that pixel position to a document model position (text range):
+    uno::Reference<frame::XModel2> xModel(mxComponent, uno::UNO_QUERY);
+    uno::Reference<container::XEnumeration> xControllers = xModel->getControllers();
+    uno::Reference<text::XTextViewTextRangeSupplier> xController(xControllers->nextElement(),
+                                                                 uno::UNO_QUERY);
+    awt::Point aPoint(aPixel.getX(), aPixel.getY());
+    // Without the accompanying fix in place, this test would have crashed, because an XTextRange
+    // can't point to a graphic node.
+    uno::Reference<text::XTextRange> xTextRange
+        = xController->createTextRangeByPixelPosition(aPoint);
+
+    // Then make sure that the anchor of the image is returned:
+    const SwFrameFormats& rFormats = *pDoc->GetSpzFrameFormats();
+    const SwFrameFormat* pFormat = rFormats[0];
+    SwPosition aAnchorPos(*pFormat->GetAnchor().GetContentAnchor());
+    auto pTextRange = dynamic_cast<SwXTextRange*>(xTextRange.get());
+    SwPaM aPaM(pDoc->GetNodes());
+    pTextRange->GetPositions(aPaM);
+    CPPUNIT_ASSERT_EQUAL(aAnchorPos, *aPaM.GetPoint());
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/uibase/uno/unotxvw.cxx b/sw/source/uibase/uno/unotxvw.cxx
index 8b82e83285e8..416defb125ef 100644
--- a/sw/source/uibase/uno/unotxvw.cxx
+++ b/sw/source/uibase/uno/unotxvw.cxx
@@ -72,6 +72,7 @@
 #include <cppuhelper/supportsservice.hxx>
 #include <cppuhelper/typeprovider.hxx>
 #include <tools/UnitConversion.hxx>
+#include <fmtanchr.hxx>
 
 using namespace ::com::sun::star;
 using namespace ::com::sun::star::uno;
@@ -535,6 +536,23 @@ SwXTextView::createTextRangeByPixelPosition(const awt::Point& rPixelPosition)
     SwWrtShell& rSh = m_pView->GetWrtShell();
     SwPosition aPosition(*rSh.GetCurrentShellCursor().GetPoint());
     rSh.GetLayout()->GetModelPositionForViewPoint(&aPosition, aLogicPoint);
+
+    if (aPosition.GetNode().IsGrfNode())
+    {
+        // The point is closest to a graphic node, look up its format.
+        const SwFrameFormat* pGraphicFormat = aPosition.GetNode().GetFlyFormat();
+        if (pGraphicFormat)
+        {
+            // Get the anchor of this format.
+            const SwFormatAnchor& rAnchor = pGraphicFormat->GetAnchor();
+            const SwPosition* pAnchor = rAnchor.GetContentAnchor();
+            if (pAnchor)
+            {
+                aPosition = *pAnchor;
+            }
+        }
+    }
+
     rtl::Reference<SwXTextRange> xRet
         = SwXTextRange::CreateXTextRange(*rSh.GetDoc(), aPosition, /*pMark=*/nullptr);