From 99c7cf2816b2d0016832552dbe6c55456ade42e0 Mon Sep 17 00:00:00 2001 From: Jonathan Clark Date: Fri, 4 Oct 2024 11:50:25 -0600 Subject: [PATCH] crashtesting: fix font-dependent infinite loop in kashida justification Commit c3c29d31d77ff93aa50634cfd51c62d12dc0f6ec (tdf#140767 Implemented Syriac justification) indirectly introduced a font-dependent infinite loop in Writer layout by relaxing some restrictions on kashida candidate positions. The infinite loop was found in tdf97806-2.odt. This change fixes an underlying logic bug in Writer, which caused Writer to attempt to apply kashida justification to non-Arabic/Syriac CTL scripts. This change also reintroduces some of the previously-lifted restrictions on kashida candidate positions. Change-Id: I783bf327f4ef0f1f6a032f0d3dddbcfd60a026fa Reviewed-on: https://gerrit.libreoffice.org/c/core/+/174503 Tested-by: Jenkins Reviewed-by: Jonathan Clark --- i18nutil/qa/cppunit/test_kashida.cxx | 2 +- i18nutil/source/utility/kashida.cxx | 19 ++++++++++++++----- sw/source/core/text/itradj.cxx | 15 ++++++++------- sw/source/core/text/porlay.cxx | 23 ++++++++++++++--------- 4 files changed, 37 insertions(+), 22 deletions(-) diff --git a/i18nutil/qa/cppunit/test_kashida.cxx b/i18nutil/qa/cppunit/test_kashida.cxx index e0526c4c8f93..fa47e4cf135f 100644 --- a/i18nutil/qa/cppunit/test_kashida.cxx +++ b/i18nutil/qa/cppunit/test_kashida.cxx @@ -86,7 +86,7 @@ void KashidaTest::testNoZwnjExpansion() CPPUNIT_ASSERT_EQUAL(sal_Int32(0), GetWordKashidaPosition(u"نویس\u200Cه"_ustr).value().nIndex); CPPUNIT_ASSERT_EQUAL(sal_Int32(1), GetWordKashidaPosition(u"متن"_ustr).value().nIndex); - CPPUNIT_ASSERT_EQUAL(sal_Int32(0), GetWordKashidaPosition(u"مت\u200Cن"_ustr).value().nIndex); + CPPUNIT_ASSERT(!GetWordKashidaPosition(u"مت\u200Cن"_ustr).has_value()); } // tdf#163105: Do not insert kashida if the position is invalid diff --git a/i18nutil/source/utility/kashida.cxx b/i18nutil/source/utility/kashida.cxx index a992e5c8e643..aff667033a11 100644 --- a/i18nutil/source/utility/kashida.cxx +++ b/i18nutil/source/utility/kashida.cxx @@ -134,6 +134,13 @@ bool CanConnectToPrev(sal_Unicode cCh, sal_Unicode cPrevCh) return bRet; } +bool isSyriacChar(sal_Unicode cCh) +{ + return (cCh >= 0x700 && cCh <= 0x74F) || (cCh >= 0x860 && cCh <= 0x86A); +} + +bool isArabicChar(sal_Unicode cCh) { return cCh >= 0x60C && cCh <= 0x6FE; } + std::optional GetWordKashidaPositionArabic(const OUString& rWord, const std::vector& pValidPositions) { @@ -283,7 +290,7 @@ GetWordKashidaPositionArabic(const OUString& rWord, const std::vector& pVa { // Reh, Zain (right joining) final form may appear in the middle of word // All others except Yeh - only at end of word - if (isRehChar(cCh) || (0x60C <= cCh && 0x6FE >= cCh && nIdx == nWordLen - 1)) + if (isRehChar(cCh) || (isArabicChar(cCh) && nIdx == nWordLen - 1)) { SAL_WARN_IF(0 == cPrevCh, "i18n", "No previous character"); // check if character is connectable to previous character, @@ -295,7 +302,8 @@ GetWordKashidaPositionArabic(const OUString& rWord, const std::vector& pVa } // 8. Try any valid position - if (nPriorityLevel >= 7 && nIdx > 0) + if (nPriorityLevel >= 7 && nIdx > 0 && isArabicChar(cPrevCh) && isArabicChar(cCh) + && !pValidPositions.empty()) { fnTryInsertBefore(7); } @@ -339,7 +347,7 @@ GetWordKashidaPositionSyriac(const OUString& rWord, const std::vector& pVa sal_Int32 nWordMidpoint = nWordLen / 2; - auto fnPositionValid = [&pValidPositions](sal_Int32 nIdx) { + auto fnPositionValid = [&pValidPositions, &rWord](sal_Int32 nIdx) { // Exclusions: // tdf#163105: Do not insert kashida if the position is invalid @@ -348,7 +356,8 @@ GetWordKashidaPositionSyriac(const OUString& rWord, const std::vector& pVa return false; } - return true; + sal_Unicode cCh = rWord[nIdx]; + return isSyriacChar(cCh); }; // End to midpoint @@ -385,7 +394,7 @@ i18nutil::GetWordKashidaPosition(const OUString& rWord, const std::vector& { auto cCh = rWord[nIdx]; - if ((cCh >= 0x700 && cCh <= 0x74F) || (cCh >= 0x860 && cCh <= 0x86A)) + if (isSyriacChar(cCh)) { // This word contains Syriac characters. return GetWordKashidaPositionSyriac(rWord, pValidPositions); diff --git a/sw/source/core/text/itradj.cxx b/sw/source/core/text/itradj.cxx index c072025e7a96..be004457b407 100644 --- a/sw/source/core/text/itradj.cxx +++ b/sw/source/core/text/itradj.cxx @@ -167,10 +167,11 @@ static bool lcl_CheckKashidaPositions(SwScriptInfo& rSI, SwTextSizeInfo& rInf, S const OUString& rWord = aScanner.GetWord(); // Fetch the set of valid positions from VCL, where possible - aValidPositions.clear(); if (SwScriptInfo::IsKashidaScriptText(rInf.GetText(), TextFrameIndex{ aScanner.GetBegin() }, TextFrameIndex{ aScanner.GetLen() })) { + aValidPositions.clear(); + rItr.SeekAndChgAttrIter(TextFrameIndex{ aScanner.GetBegin() }, rInf.GetRefDev()); vcl::text::ComplexTextLayoutFlags nOldLayout = rInf.GetRefDev()->GetLayoutMode(); @@ -179,13 +180,13 @@ static bool lcl_CheckKashidaPositions(SwScriptInfo& rSI, SwTextSizeInfo& rInf, S rInf.GetRefDev()->GetWordKashidaPositions(rWord, &aValidPositions); rInf.GetRefDev()->SetLayoutMode(nOldLayout); - } - auto stKashidaPos = i18nutil::GetWordKashidaPosition(rWord, aValidPositions); - if (stKashidaPos.has_value()) - { - TextFrameIndex nNewKashidaPos{ aScanner.GetBegin() + stKashidaPos->nIndex }; - aNewKashidaPositions.push_back(nNewKashidaPos); + auto stKashidaPos = i18nutil::GetWordKashidaPosition(rWord, aValidPositions); + if (stKashidaPos.has_value()) + { + TextFrameIndex nNewKashidaPos{ aScanner.GetBegin() + stKashidaPos->nIndex }; + aNewKashidaPositions.push_back(nNewKashidaPos); + } } } diff --git a/sw/source/core/text/porlay.cxx b/sw/source/core/text/porlay.cxx index d433c3bbd3cd..39a260ac0ef8 100644 --- a/sw/source/core/text/porlay.cxx +++ b/sw/source/core/text/porlay.cxx @@ -1525,16 +1525,21 @@ void SwScriptInfo::InitScriptInfo(const SwTextNode& rNode, // the search has to be performed on a per word base while ( aScanner.NextWord() ) { - const OUString& rWord = aScanner.GetWord(); - auto stKashidaPos = i18nutil::GetWordKashidaPosition(rWord); - - if (stKashidaPos.has_value()) + if (SwScriptInfo::IsKashidaScriptText(rText, TextFrameIndex{ aScanner.GetBegin() }, + TextFrameIndex{ aScanner.GetLen() })) { - // Only populate kashida positions for the invalidated tail - TextFrameIndex nNewKashidaPos{aScanner.GetBegin() + stKashidaPos->nIndex}; - if(nNewKashidaPos >= nLastKashida) { - m_Kashida.insert(m_Kashida.begin() + nCntKash, nNewKashidaPos); - nCntKash++; + const OUString& rWord = aScanner.GetWord(); + auto stKashidaPos = i18nutil::GetWordKashidaPosition(rWord); + + if (stKashidaPos.has_value()) + { + // Only populate kashida positions for the invalidated tail + TextFrameIndex nNewKashidaPos{ aScanner.GetBegin() + stKashidaPos->nIndex }; + if (nNewKashidaPos >= nLastKashida) + { + m_Kashida.insert(m_Kashida.begin() + nCntKash, nNewKashidaPos); + nCntKash++; + } } } } // end of kashida search