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(-)
New commits:
commit 99c7cf2816b2d0016832552dbe6c55456ade42e0
Author: Jonathan Clark <jonat...@libreoffice.org>
AuthorDate: Fri Oct 4 11:50:25 2024 -0600
Commit: Jonathan Clark <jonat...@libreoffice.org>
CommitDate: Sat Oct 5 00:05:05 2024 +0200
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 <jonat...@libreoffice.org>
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<i18nutil::KashidaPosition>
GetWordKashidaPositionArabic(const OUString& rWord, const std::vector<bool>&
pValidPositions)
{
@@ -283,7 +290,7 @@ GetWordKashidaPositionArabic(const OUString& rWord, const
std::vector<bool>& 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<bool>& 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<bool>& 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<bool>& 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<bool>&
{
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
