sw/inc/ToxLinkProcessor.hxx | 9 +++++-- sw/qa/core/test_ToxLinkProcessor.cxx | 38 ++++++++++++++------------------ sw/source/core/tox/ToxLinkProcessor.cxx | 18 +++++++-------- 3 files changed, 32 insertions(+), 33 deletions(-)
New commits: commit 68f7be01a78a5bad56ddd94c04764131676d6cc0 Author: Michael Stahl <mst...@redhat.com> Date: Wed Jan 25 17:12:42 2017 +0100 sw: revert ToX hyperlinks to LO 4.3 behaviour Commit 94b296d5416dd71d721ad16216b50bce79e3dc04 changed this to potentially insert multiple overlapping hyperlink items. Unfortunately Writer can only have one RES_TXTATR_INETFMT on any given position in the paragraph, so the hyperlink hints inevitably overwrite each other. Revert this to do it the same way as the old code in LO 4.3 did: match the last unmatched link-start with the first link-end, and ignore all the link-start before the matching one, as well as the link-end after the matching one. This should prevent surprising formatting changes on index update, including entries spontaneously turning green, and there is no reason why the result of the new way is objectively better than the old one. Change-Id: I55be9c212c473908428fa8bd6487d136343fe852 diff --git a/sw/inc/ToxLinkProcessor.hxx b/sw/inc/ToxLinkProcessor.hxx index 7fceef1..78bb7fb 100644 --- a/sw/inc/ToxLinkProcessor.hxx +++ b/sw/inc/ToxLinkProcessor.hxx @@ -82,7 +82,7 @@ private: std::vector<std::unique_ptr<ClosedLink>> m_ClosedLinks; - std::vector<std::unique_ptr<StartedLink>> m_StartedLinks; + std::unique_ptr<StartedLink> m_pStartedLink; friend class ::ToxLinkProcessorTest; }; diff --git a/sw/qa/core/test_ToxLinkProcessor.cxx b/sw/qa/core/test_ToxLinkProcessor.cxx index eb4618e..d2d4721 100644 --- a/sw/qa/core/test_ToxLinkProcessor.cxx +++ b/sw/qa/core/test_ToxLinkProcessor.cxx @@ -30,17 +30,15 @@ using namespace sw; class ToxLinkProcessorTest : public test::BootstrapFixture { void NoExceptionIsThrownIfTooManyLinksAreClosed(); - void AddingAndClosingTwoLinksResultsInTwoClosedLinks(); + void AddingAndClosingTwoOverlappingLinksResultsInOneClosedLink(); void LinkIsCreatedCorrectly(); void LinkSequenceIsPreserved(); - void StandardOpenLinkIsAddedWhenMoreLinksThanAvaiableAreClosed(); CPPUNIT_TEST_SUITE(ToxLinkProcessorTest); CPPUNIT_TEST(NoExceptionIsThrownIfTooManyLinksAreClosed); - CPPUNIT_TEST(AddingAndClosingTwoLinksResultsInTwoClosedLinks); + CPPUNIT_TEST(AddingAndClosingTwoOverlappingLinksResultsInOneClosedLink); CPPUNIT_TEST(LinkIsCreatedCorrectly); CPPUNIT_TEST(LinkSequenceIsPreserved); - CPPUNIT_TEST(StandardOpenLinkIsAddedWhenMoreLinksThanAvaiableAreClosed); CPPUNIT_TEST_SUITE_END(); public: void setUp() override { @@ -71,30 +69,28 @@ ToxLinkProcessorTest::NoExceptionIsThrownIfTooManyLinksAreClosed() sut.CloseLink(1, URL_1); // fdo#85872 actually it turns out the UI does something like this // so an exception must not be thrown! - sut.CloseLink(1, URL_1); + // should not succeed either (for backward compatibility) + sut.CloseLink(2, URL_1); + CPPUNIT_ASSERT_EQUAL(1u, static_cast<unsigned>(sut.m_ClosedLinks.size())); + CPPUNIT_ASSERT_EQUAL(1u, static_cast<unsigned>(sut.m_ClosedLinks.at(0)->mEndTextPos)); + CPPUNIT_ASSERT_MESSAGE("no links are open", sut.m_pStartedLink == nullptr); } void -ToxLinkProcessorTest::StandardOpenLinkIsAddedWhenMoreLinksThanAvaiableAreClosed() -{ - ToxLinkProcessor sut; - sut.StartNewLink(0, STYLE_NAME_1); - sut.CloseLink(1, URL_1); - sut.CloseLink(1, URL_1); - CPPUNIT_ASSERT_EQUAL(2u, static_cast<unsigned>(sut.m_ClosedLinks.size())); - CPPUNIT_ASSERT_EQUAL(1u, static_cast<unsigned>(sut.m_ClosedLinks.at(1)->mEndTextPos)); -} - -void -ToxLinkProcessorTest::AddingAndClosingTwoLinksResultsInTwoClosedLinks() +ToxLinkProcessorTest::AddingAndClosingTwoOverlappingLinksResultsInOneClosedLink() { ToxLinkProcessor sut; sut.StartNewLink(0, STYLE_NAME_1); sut.StartNewLink(0, STYLE_NAME_2); sut.CloseLink(1, URL_1); + // this should not cause an error, and should not succeed either + // (for backward compatibility) sut.CloseLink(1, URL_2); - CPPUNIT_ASSERT_EQUAL(2u, static_cast<unsigned>(sut.m_ClosedLinks.size())); - CPPUNIT_ASSERT_MESSAGE("no links are open", sut.m_StartedLinks.empty()); + CPPUNIT_ASSERT_EQUAL(1u, static_cast<unsigned>(sut.m_ClosedLinks.size())); + CPPUNIT_ASSERT_MESSAGE("no links are open", sut.m_pStartedLink == nullptr); + // backward compatibility: the last start is closed by the first end + CPPUNIT_ASSERT_EQUAL(STYLE_NAME_2, sut.m_ClosedLinks[0]->mINetFormat.GetINetFormat()); + CPPUNIT_ASSERT_EQUAL(URL_1, sut.m_ClosedLinks[0]->mINetFormat.GetValue()); } class ToxLinkProcessorWithOverriddenObtainPoolId : public ToxLinkProcessor { @@ -131,10 +127,10 @@ ToxLinkProcessorTest::LinkSequenceIsPreserved() // obtainpoolid needs to be overridden to check what we are ToxLinkProcessorWithOverriddenObtainPoolId sut; - sut.StartNewLink(0, STYLE_NAME_1); sut.StartNewLink(0, STYLE_NAME_2); sut.CloseLink(1, URL_2); - sut.CloseLink(1, URL_1); + sut.StartNewLink(1, STYLE_NAME_1); + sut.CloseLink(2, URL_1); // check first closed element CPPUNIT_ASSERT_EQUAL_MESSAGE("Style is stored correctly in link", diff --git a/sw/source/core/tox/ToxLinkProcessor.cxx b/sw/source/core/tox/ToxLinkProcessor.cxx index cf0b24a..f24b1a3 100644 --- a/sw/source/core/tox/ToxLinkProcessor.cxx +++ b/sw/source/core/tox/ToxLinkProcessor.cxx @@ -21,19 +21,18 @@ namespace sw { void ToxLinkProcessor::StartNewLink(sal_Int32 startPosition, const OUString& characterStyle) { - m_StartedLinks.push_back(o3tl::make_unique<StartedLink>( - startPosition, characterStyle)); + SAL_INFO_IF(m_pStartedLink, "sw.core", "ToxLinkProcessor: LS without LE"); + m_pStartedLink = o3tl::make_unique<StartedLink>( + startPosition, characterStyle); } void ToxLinkProcessor::CloseLink(sal_Int32 endPosition, const OUString& url) { - StartedLink const startedLink( (m_StartedLinks.empty()) - ? StartedLink(0, SW_RES(STR_POOLCHR_TOXJUMP)) - : *m_StartedLinks.back() ); - if (!m_StartedLinks.empty()) + if (m_pStartedLink == nullptr) { - m_StartedLinks.pop_back(); + SAL_INFO("sw.core", "ToxLinkProcessor: LE without LS"); + return; } if (url.isEmpty()) { @@ -41,14 +40,15 @@ ToxLinkProcessor::CloseLink(sal_Int32 endPosition, const OUString& url) } std::unique_ptr<ClosedLink> pClosedLink( - new ClosedLink(url, startedLink.mStartPosition, endPosition)); + new ClosedLink(url, m_pStartedLink->mStartPosition, endPosition)); - const OUString& characterStyle = startedLink.mCharacterStyle; + const OUString& characterStyle = m_pStartedLink->mCharacterStyle; sal_uInt16 poolId = ObtainPoolId(characterStyle); pClosedLink->mINetFormat.SetVisitedFormatAndId(characterStyle, poolId); pClosedLink->mINetFormat.SetINetFormatAndId(characterStyle, poolId); m_ClosedLinks.push_back(std::move(pClosedLink)); + m_pStartedLink.reset(); } sal_uInt16 commit 4a43ffcef946067c5b3992a00c765a36804a194f Author: Michael Stahl <mst...@redhat.com> Date: Wed Jan 25 16:42:16 2017 +0100 sw: stop swapping start and end position of links in ToX This causes: sw/source/core/txtnode/thints.cxx:3198: +SwpHints::Insert: invalid hint, end < start Change-Id: I933c790127ab1469bb57c4d288dbb49be16ace19 diff --git a/sw/inc/ToxLinkProcessor.hxx b/sw/inc/ToxLinkProcessor.hxx index 699c0ec..7fceef1 100644 --- a/sw/inc/ToxLinkProcessor.hxx +++ b/sw/inc/ToxLinkProcessor.hxx @@ -69,8 +69,11 @@ private: * A link is closed if it has both a start and an end token. */ struct ClosedLink { - ClosedLink(const OUString& url, sal_Int32 startPosition, sal_Int32 endPosition) : - mINetFormat(url, OUString()), mStartTextPos(endPosition), mEndTextPos(startPosition) { + ClosedLink(const OUString& url, sal_Int32 startPosition, sal_Int32 endPosition) + : mINetFormat(url, OUString()) + , mStartTextPos(startPosition) + , mEndTextPos(endPosition) + { } SwFormatINetFormat mINetFormat; sal_Int32 mStartTextPos; diff --git a/sw/qa/core/test_ToxLinkProcessor.cxx b/sw/qa/core/test_ToxLinkProcessor.cxx index 0a15982..eb4618e 100644 --- a/sw/qa/core/test_ToxLinkProcessor.cxx +++ b/sw/qa/core/test_ToxLinkProcessor.cxx @@ -82,7 +82,7 @@ ToxLinkProcessorTest::StandardOpenLinkIsAddedWhenMoreLinksThanAvaiableAreClosed( sut.CloseLink(1, URL_1); sut.CloseLink(1, URL_1); CPPUNIT_ASSERT_EQUAL(2u, static_cast<unsigned>(sut.m_ClosedLinks.size())); - CPPUNIT_ASSERT_EQUAL(0u, static_cast<unsigned>(sut.m_ClosedLinks.at(1)->mEndTextPos)); + CPPUNIT_ASSERT_EQUAL(1u, static_cast<unsigned>(sut.m_ClosedLinks.at(1)->mEndTextPos)); } void _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits