tdf#150719 PPTX import: fix hyperlink format (lost underline)

Hypertext lost its formatting partially: e.g. underline
character setting lost, except on the last word.

Follow-up to commit commit a761a51d9db3a2771ca9fd6ab233c513aa5d8ecf
"tdf#149311 PPTX export: fix internal hyperlink on texts".

Clean-up of commit 855a56fea4561135a63cb729d7a625a950b210e7
"tdf#148965 PPTX import: fix internal hyperlinks on shapes" and
commit cec1f712c87e557e1b7313e0dbef4a635f69d953
"tdf#144918 PPTX import: fix internal hyperlink on shapes" and
commit 7eb0e52527e729a21973e70d5be8e0a6779ec748
"tdf#142648 PPTX: import long slide names to avoid broken link export".

Change-Id: I1de8b06361c7b9529a70a039e194db88460cc27b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/138669
Tested-by: László Németh <nemeth@numbertext.org>
Reviewed-by: László Németh <nemeth@numbertext.org>

12 files changed, 63 insertions, 181 deletions

diff --git a/include/oox/core/xmlfilterbase.hxx b/include/oox/core/xmlfilterbase.hxx
index 35312fda53ab..89a7994a904b 100644
--- a/include/oox/core/xmlfilterbase.hxx
+++ b/include/oox/core/xmlfilterbase.hxx
@@ -75,13 +75,6 @@ namespace oox::core {
 class FragmentHandler;
 class FastParser;
 
-struct TextField {
-    css::uno::Reference< css::text::XText >       xText;
-    css::uno::Reference< css::text::XTextCursor > xTextCursor;
-    css::uno::Reference< css::text::XTextField >  xTextField;
-};
-typedef std::vector< TextField > TextFieldStack;
-
 struct XmlFilterBaseImpl;
 
 using ShapePairs
@@ -183,9 +176,6 @@ public:
      */
     OUString     addRelation( const css::uno::Reference< css::io::XOutputStream >& rOutputStream, const OUString& rType, std::u16string_view rTarget, bool bExternal = false );
 
-    /** Returns a stack of used textfields, used by the pptx importer to replace links to slidepages with the real page name */
-    TextFieldStack& getTextFieldStack() const;
-
     /** Opens and returns the specified output stream from the base storage with specified media type.
 
         @param rStreamName
diff --git a/include/oox/drawingml/shape.hxx b/include/oox/drawingml/shape.hxx
index 246a964fc2a6..e481b98c3f6e 100644
--- a/include/oox/drawingml/shape.hxx
+++ b/include/oox/drawingml/shape.hxx
@@ -140,9 +140,6 @@ public:
     void                            setConnectorShape(bool bConnector) { mbConnector = bConnector; }
     bool                            isConnectorShape() const { return mbConnector; }
 
-    void                            setBookmark(bool bBookmark) { mbHasBookmark = bBookmark; }
-    bool                            hasBookmark() const { return mbHasBookmark; }
-
     Shape3DProperties&              get3DProperties() { return *mp3DPropertiesPtr; }
     const Shape3DProperties&        get3DProperties() const { return *mp3DPropertiesPtr; }
 
@@ -408,9 +405,6 @@ private:
     // Is this a connector shape?
     bool mbConnector = false;
 
-    // Is shape has bookmark?
-    bool mbHasBookmark = false;
-
     // temporary space for DiagramHelper in preparation for collecting data
     // Note: I tried to use a unique_ptr here, but existing constructor func does not allow that
     svx::diagram::IDiagramHelper* mpDiagramHelper;
diff --git a/include/oox/ppt/presentationfragmenthandler.hxx b/include/oox/ppt/presentationfragmenthandler.hxx
index a9bb5bb67a77..7ac929ec555b 100644
--- a/include/oox/ppt/presentationfragmenthandler.hxx
+++ b/include/oox/ppt/presentationfragmenthandler.hxx
@@ -52,6 +52,7 @@ private:
     void importSlide(sal_uInt32 nSlide, bool bFirstSlide, bool bImportNotes);
     void saveThemeToGrabBag(const oox::drawingml::ThemePtr& pThemePtr, sal_Int32 nThemeIdx);
     void importCustomSlideShow(std::vector<CustomShow>& rCustomShowList);
+    static void importSlideNames(::oox::core::XmlFilterBase& rFilter, const std::vector<SlidePersistPtr>& rSlidePersist);
 
     std::vector< OUString > maSlideMasterVector;
     std::vector< OUString > maSlidesVector;
diff --git a/include/oox/ppt/slidepersist.hxx b/include/oox/ppt/slidepersist.hxx
index 659dea4c85a4..05ecb0094856 100644
--- a/include/oox/ppt/slidepersist.hxx
+++ b/include/oox/ppt/slidepersist.hxx
@@ -127,9 +127,6 @@ public:
 
     void createConnectorShapeConnection();
 
-    void  addURLShapeId(const OUString& rShapeId) { maURLShapeId.push_back(rShapeId); }
-    std::vector<OUString>& getURLShapeId() { return maURLShapeId; }
-
 private:
     OUString                                                                maPath;
     OUString                                                                maLayoutPath;
@@ -163,7 +160,6 @@ private:
     CommentAuthorList                                                       maCommentAuthors;
 
     std::vector<OUString>                                                   maConnectorShapeId;
-    std::vector<OUString>                                                   maURLShapeId;
 };
 
 }
diff --git a/oox/source/core/xmlfilterbase.cxx b/oox/source/core/xmlfilterbase.cxx
index d415a739fe3e..fe6ce6dc24fb 100644
--- a/oox/source/core/xmlfilterbase.cxx
+++ b/oox/source/core/xmlfilterbase.cxx
@@ -175,7 +175,6 @@ struct XmlFilterBaseImpl
 
     FastParser                     maFastParser;
     RelationsMap                   maRelationsMap;
-    TextFieldStack                 maTextFieldStack;
     const NamespaceMap&            mrNamespaceMap;
     NamedShapePairs* mpDiagramFontHeights = nullptr;
 
@@ -507,11 +506,6 @@ FSHelperPtr XmlFilterBase::openFragmentStreamWithSerializer( const OUString& rSt
     return std::make_shared<FastSerializerHelper>( openFragmentStream( rStreamName, rMediaType ), bWriteHeader );
 }
 
-TextFieldStack& XmlFilterBase::getTextFieldStack() const
-{
-    return mxImpl->maTextFieldStack;
-}
-
 namespace {
 
 OUString lclAddRelation( const Reference< XRelationshipAccess >& rRelations, sal_Int32 nId, const OUString& rType, std::u16string_view rTarget, bool bExternal )
diff --git a/oox/source/drawingml/hyperlinkcontext.cxx b/oox/source/drawingml/hyperlinkcontext.cxx
index ab52c0f5e397..920d37609f8f 100644
--- a/oox/source/drawingml/hyperlinkcontext.cxx
+++ b/oox/source/drawingml/hyperlinkcontext.cxx
@@ -29,6 +29,8 @@
 #include <oox/token/properties.hxx>
 #include <oox/token/tokens.hxx>
 #include <o3tl/string_view.hxx>
+#include <ooxresid.hxx>
+#include <strings.hrc>
 
 using namespace ::oox::core;
 using namespace ::com::sun::star::uno;
@@ -122,7 +124,7 @@ HyperLinkContext::HyperLinkContext( ContextHandler2Helper const & rParent,
                     {
                         const OUString aSlideType( sHref.copy( 0, nIndex2 ) );
                         if ( aSlideType.match( "slide" ) )
-                            sURL = "#Slide " + OUString::number( nPageNumber );
+                            sURL = "#" + URLResId(STR_SLIDE_NAME) + " " + OUString::number( nPageNumber );
                         else if ( aSlideType.match( "notesSlide" ) )
                             sURL = "#Notes " + OUString::number( nPageNumber );
 //                      else: todo for other types such as notesMaster or slideMaster as they can't be referenced easily
diff --git a/oox/source/drawingml/textrun.cxx b/oox/source/drawingml/textrun.cxx
index 339d50bb4ff0..6bfc3701fedb 100644
--- a/oox/source/drawingml/textrun.cxx
+++ b/oox/source/drawingml/textrun.cxx
@@ -163,12 +163,6 @@ sal_Int32 TextRun::insertAt(
 
                 PropertySet aFieldTextPropSet( xTextFieldCursor );
                 aTextCharacterProps.pushToPropSet( aFieldTextPropSet, rFilterBase );
-
-                oox::core::TextField aTextField;
-                aTextField.xText = xText;
-                aTextField.xTextCursor = xTextFieldCursor;
-                aTextField.xTextField = xField;
-                rFilterBase.getTextFieldStack().push_back( aTextField );
             }
             else
             {
diff --git a/oox/source/ppt/pptshape.cxx b/oox/source/ppt/pptshape.cxx
index ffa337b4de9a..be7046c7498e 100644
--- a/oox/source/ppt/pptshape.cxx
+++ b/oox/source/ppt/pptshape.cxx
@@ -447,47 +447,6 @@ void PPTShape::addShape(
                 setMasterTextListStyle( aMasterTextListStyle );
 
             Reference< XShape > xShape( createAndInsert( rFilterBase, sServiceName, pTheme, rxShapes, bClearText, bool(mpPlaceholder), aTransformation, getFillProperties() ) );
-            // if exists and not duplicated, try to use the title text as slide name to help its re-use on UI
-            if (!rSlidePersist.isMasterPage() && rSlidePersist.getPage().is() && (mnSubType == XML_title || mnSubType == XML_ctrTitle))
-            {
-                try
-                {
-                    sal_Int32 nCount = 1;
-                    OUString aTitleText;
-                    Reference<XTextRange> xText(xShape, UNO_QUERY_THROW);
-                    aTitleText = xText->getString();
-                    Reference<drawing::XDrawPagesSupplier> xDPS(rFilterBase.getModel(), uno::UNO_QUERY_THROW);
-                    Reference<drawing::XDrawPages> xDrawPages(xDPS->getDrawPages(), uno::UNO_SET_THROW);
-                    sal_uInt32 nMaxPages = xDrawPages->getCount();
-                    // just a magic value but we don't want to drop out slide names which are too long
-                    if (aTitleText.getLength() > 63)
-                        aTitleText = aTitleText.copy(0, 63);
-                    bool bUseTitleAsSlideName = !aTitleText.isEmpty();
-                    // check duplicated title name
-                    if (bUseTitleAsSlideName)
-                    {
-                        for (sal_uInt32 nPage = 0; nPage < nMaxPages; ++nPage)
-                        {
-                            Reference<XDrawPage> xDrawPage(xDrawPages->getByIndex(nPage), uno::UNO_QUERY);
-                            Reference<container::XNamed> xNamed(xDrawPage, UNO_QUERY_THROW);
-                            OUString sRest;
-                            if (xNamed->getName().startsWith(aTitleText, &sRest)
-                                && (sRest.isEmpty()
-                                    || (sRest.startsWith(" (") && sRest.endsWith(")")
-                                        && o3tl::toInt32(sRest.subView(2, sRest.getLength() - 3)) > 0)))
-                                nCount++;
-                        }
-                        Reference<container::XNamed> xName(rSlidePersist.getPage(), UNO_QUERY_THROW);
-                        xName->setName(
-                            aTitleText
-                            + (nCount == 1 ? OUString("") : " (" + OUString::number(nCount) + ")"));
-                    }
-                }
-                catch (uno::Exception&)
-                {
-
-                }
-            }
 
             // Apply text properties on placeholder text inside this placeholder shape
             if (meShapeLocation == Slide && mpPlaceholder && getTextBody() && getTextBody()->isEmpty())
@@ -613,14 +572,12 @@ void PPTShape::addShape(
                     // so check here if it's a bookmark or a document
                     if (meClickAction == ClickAction_BOOKMARK)
                     {
-                        sal_Int32 nSplitPos;
                         if (!sURL.startsWith("#"))
                             meClickAction = ClickAction_DOCUMENT;
-                        else if (-1 != (nSplitPos = sURL.indexOf( ' ' )))
+                        else
                         {
-                            setBookmark(true);
-                            // reuse slide number from '#Slide [Num]' or "#Notes [Num]"
-                            sURL = OUString::Concat("#page") + sURL.subView(nSplitPos);
+                            sURL = OUString::Concat("page")
+                                   + sURL.subView(sURL.lastIndexOf(' ') + 1);
                         }
                         nPropertyCount += 1;
                     }
diff --git a/oox/source/ppt/presentationfragmenthandler.cxx b/oox/source/ppt/presentationfragmenthandler.cxx
index 2db197ce3a90..c3bfc9cab9e2 100644
--- a/oox/source/ppt/presentationfragmenthandler.cxx
+++ b/oox/source/ppt/presentationfragmenthandler.cxx
@@ -108,110 +108,49 @@ PresentationFragmentHandler::~PresentationFragmentHandler() noexcept
 {
 }
 
-static void lcl_setBookmark(uno::Reference<drawing::XShape>& rShape,
-                            std::vector<SlidePersistPtr>& rSlidePersist)
+void PresentationFragmentHandler::importSlideNames(XmlFilterBase& rFilter, const std::vector<SlidePersistPtr>& rSlidePersist)
 {
-    OUString aBookmark;
-    static const OUStringLiteral sSlideName = u"#page";
-    uno::Reference<beans::XPropertySet> xPropSet(rShape, uno::UNO_QUERY);
-    xPropSet->getPropertyValue("Bookmark") >>= aBookmark;
-    if (aBookmark.startsWith(sSlideName))
+    sal_Int32 nMaxPages = rSlidePersist.size();
+    for (sal_Int32 nPage = 0; nPage < nMaxPages; nPage++)
     {
-        sal_Int32 nPageNumber = o3tl::toInt32(aBookmark.subView(sSlideName.getLength()));
-        Reference<XDrawPage> xDrawPage(rSlidePersist[nPageNumber - 1]->getPage());
-        Reference<container::XNamed> xNamed(xDrawPage, UNO_QUERY_THROW);
-        aBookmark = xNamed->getName();
-        xPropSet->setPropertyValue("Bookmark", Any(aBookmark));
-    }
-}
-
-static void ResolveShapeBookmark(std::vector<SlidePersistPtr>& rSlidePersist)
-{
-    sal_Int32 nPageCount = rSlidePersist.size();
-    for (sal_Int32 nPage = 0; nPage < nPageCount; ++nPage)
-    {
-        if (!rSlidePersist[nPage]->getURLShapeId().empty())
-        {
-            auto aShapeMap = rSlidePersist[nPage]->getShapeMap();
-            sal_Int32 nCount = rSlidePersist[nPage]->getURLShapeId().size();
-            for (sal_Int32 i = 0; i < nCount; i++)
-            {
-                OUString sId = rSlidePersist[nPage]->getURLShapeId()[i];
-                uno::Reference<drawing::XShape> xShape(aShapeMap[sId]->getXShape(), uno::UNO_QUERY);
-                Reference<XShapes> xShapes(xShape, UNO_QUERY);
-                if (xShapes.is()) // group shape
-                {
-                    for (sal_Int32 j = 0; j < xShapes->getCount(); j++)
-                    {
-                        uno::Reference<drawing::XShape> xGroupedShape(xShapes->getByIndex(j),
-                                                                      uno::UNO_QUERY);
-                        lcl_setBookmark(xGroupedShape, rSlidePersist);
-                    }
-                }
-                else
-                    lcl_setBookmark(xShape, rSlidePersist);
-            }
-        }
-    }
-}
-
-static void ResolveTextFields( XmlFilterBase const & rFilter )
-{
-    const oox::core::TextFieldStack& rTextFields = rFilter.getTextFieldStack();
-    if ( rTextFields.empty() )
-        return;
-
-    const Reference< frame::XModel >& xModel( rFilter.getModel() );
-    for (auto const& textField : rTextFields)
-    {
-        static const OUStringLiteral sURL = u"URL";
-        Reference< drawing::XDrawPagesSupplier > xDPS( xModel, uno::UNO_QUERY_THROW );
-        Reference< drawing::XDrawPages > xDrawPages( xDPS->getDrawPages(), uno::UNO_SET_THROW );
-
-        const oox::core::TextField& rTextField( textField );
-        Reference< XPropertySet > xPropSet( rTextField.xTextField, UNO_QUERY );
-        Reference< XPropertySetInfo > xPropSetInfo( xPropSet->getPropertySetInfo() );
-        if ( xPropSetInfo->hasPropertyByName( sURL ) )
+        auto aShapeMap = rSlidePersist[nPage]->getShapeMap();
+        auto aIter = std::find_if(aShapeMap.begin(), aShapeMap.end(),
+                                  [](const std::pair<OUString, ShapePtr>& element) {
+                                      auto pShapePtr = element.second;
+                                      return (pShapePtr
+                                              && (pShapePtr->getSubType() == XML_title
+                                                  || pShapePtr->getSubType() == XML_ctrTitle));
+                                  });
+        if (aIter != aShapeMap.end())
         {
-            OUString aURL;
-            if ( xPropSet->getPropertyValue( sURL ) >>= aURL )
+            OUString aTitleText;
+            Reference<text::XTextRange> xText(aIter->second->getXShape(), UNO_QUERY_THROW);
+            aTitleText = xText->getString();
+            // just a magic value but we don't want to drop out slide names which are too long
+            if (aTitleText.getLength() > 63)
+                aTitleText = aTitleText.copy(0, 63);
+            bool bUseTitleAsSlideName = !aTitleText.isEmpty();
+            // check duplicated title name
+            if (bUseTitleAsSlideName)
             {
-                static const OUStringLiteral sSlide = u"#Slide ";
-                static const OUStringLiteral sNotes = u"#Notes ";
-                bool bNotes = false;
-                sal_Int32 nPageNumber = 0;
-                if ( aURL.match( sSlide ) )
-                    nPageNumber = o3tl::toInt32(aURL.subView( sSlide.getLength() ));
-                else if ( aURL.match( sNotes ) )
-                {
-                    nPageNumber = o3tl::toInt32(aURL.subView( sNotes.getLength() ));
-                    bNotes = true;
-                }
-                if ( nPageNumber )
+                sal_Int32 nCount = 1;
+                Reference<XDrawPagesSupplier> xDPS(rFilter.getModel(), UNO_QUERY_THROW);
+                Reference<XDrawPages> xDrawPages(xDPS->getDrawPages(), UNO_SET_THROW);
+                for (sal_Int32 i = 0; i < nPage; ++i)
                 {
-                    try
-                    {
-                        Reference< XDrawPage > xDrawPage;
-                        xDrawPages->getByIndex( nPageNumber - 1 ) >>= xDrawPage;
-                        if ( bNotes )
-                        {
-                            Reference< css::presentation::XPresentationPage > xPresentationPage( xDrawPage, UNO_QUERY_THROW );
-                            xDrawPage = xPresentationPage->getNotesPage();
-                        }
-                        Reference< container::XNamed > xNamed( xDrawPage, UNO_QUERY_THROW );
-                        if (!xNamed->getName().startsWith("page"))
-                            aURL = "#" + xNamed->getName();
-                        else
-                            aURL = "#" + URLResId(STR_SLIDE_NAME) + " " + OUString::number(nPageNumber);
-                        xPropSet->setPropertyValue( sURL, Any( aURL ) );
-                        Reference< text::XTextContent > xContent( rTextField.xTextField);
-                        Reference< text::XTextRange > xTextRange = rTextField.xTextCursor;
-                        rTextField.xText->insertTextContent( xTextRange, xContent, true );
-                    }
-                    catch( uno::Exception& )
-                    {
-                    }
+                    Reference<XDrawPage> xDrawPage(xDrawPages->getByIndex(i), UNO_QUERY);
+                    Reference<container::XNamed> xNamed(xDrawPage, UNO_QUERY_THROW);
+                    OUString sRest;
+                    if (xNamed->getName().startsWith(aTitleText, &sRest)
+                        && (sRest.isEmpty()
+                            || (sRest.startsWith(" (") && sRest.endsWith(")")
+                                && o3tl::toInt32(sRest.subView(2, sRest.getLength() - 3)) > 0)))
+                        nCount++;
                 }
+                Reference<container::XNamed> xName(rSlidePersist[nPage]->getPage(), UNO_QUERY_THROW);
+                xName->setName(
+                    aTitleText
+                    + (nCount == 1 ? OUString("") : " (" + OUString::number(nCount) + ")"));
             }
         }
     }
@@ -602,8 +541,7 @@ void PresentationFragmentHandler::finalizeImport()
                 importSlide(elem, !nPagesImported, bImportNotesPages);
                 nPagesImported++;
             }
-            ResolveTextFields( rFilter );
-            ResolveShapeBookmark(rFilter.getDrawPages());
+            importSlideNames( rFilter, rFilter.getDrawPages());
             if (!maCustomShowList.empty())
                 importCustomSlideShow(maCustomShowList);
         }
diff --git a/oox/source/ppt/slidepersist.cxx b/oox/source/ppt/slidepersist.cxx
index 4b590e38cbd0..1e7461fa5f49 100644
--- a/oox/source/ppt/slidepersist.cxx
+++ b/oox/source/ppt/slidepersist.cxx
@@ -159,8 +159,6 @@ void SlidePersist::createXShapes( XmlFilterBase& rFilterBase )
                             maConnectorShapeId.push_back(pPPTShape->getChildren()[i]->getId());
                     }
                 }
-                if (pPPTShape->hasBookmark())
-                    addURLShapeId(pPPTShape->getId());
             }
             else
                 child->addShape( rFilterBase, getTheme().get(), xShapes, aTransformation, maShapesPtr->getFillProperties(), &getShapeMap() );
diff --git a/sd/qa/unit/data/pptx/tdf150719.pptx b/sd/qa/unit/data/pptx/tdf150719.pptx
new file mode 100644
index 000000000000..d8aedf6b75a8
--- /dev/null
+++ b/sd/qa/unit/data/pptx/tdf150719.pptx
diff --git a/sd/qa/unit/import-tests.cxx b/sd/qa/unit/import-tests.cxx
index 6c2d3084fb23..aa6811a33c91 100644
--- a/sd/qa/unit/import-tests.cxx
+++ b/sd/qa/unit/import-tests.cxx
@@ -83,6 +83,7 @@ public:
     virtual void setUp() override;
 
     void testDocumentLayout();
+    void testTdf150719();
     void testTdf149314();
     void testTdf149124();
     void testTdf148965();
@@ -154,6 +155,7 @@ public:
     CPPUNIT_TEST_SUITE(SdImportTest);
 
     CPPUNIT_TEST(testDocumentLayout);
+    CPPUNIT_TEST(testTdf150719);
     CPPUNIT_TEST(testTdf149314);
     CPPUNIT_TEST(testTdf149124);
     CPPUNIT_TEST(testTdf148965);
@@ -303,6 +305,22 @@ void SdImportTest::testDocumentLayout()
     }
 }
 
+void SdImportTest::testTdf150719()
+{
+    ::sd::DrawDocShellRef xDocShRef
+        = loadURL(m_directories.getURLFromSrc(u"/sd/qa/unit/data/pptx/tdf150719.pptx"), PPTX);
+
+    uno::Reference<beans::XPropertySet> xShape(getShapeFromPage(0, 0, xDocShRef));
+    uno::Reference<text::XTextRange> const xParagraph(getParagraphFromShape(0, xShape));
+    uno::Reference<text::XTextRange> xRun(getRunFromParagraph(1, xParagraph));
+    uno::Reference<beans::XPropertySet> xPropSet1(xRun, uno::UNO_QUERY_THROW);
+    sal_Int16 nUnderline;
+    xPropSet1->getPropertyValue("CharUnderline") >>= nUnderline;
+    CPPUNIT_ASSERT_EQUAL_MESSAGE("The underline is missing!", sal_Int16(1), nUnderline);
+
+    xDocShRef->DoClose();
+}
+
 void SdImportTest::testTdf149314()
 {
     sd::DrawDocShellRef xDocShRef