From e74fb5effe6542340af85d50970787c133a911af Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 25 Aug 2017 01:42:51 +0200 Subject: [PATCH 1/3] Account for the last position in wxMSW wxTextCtrl There is a valid position after the last character of the text in wxTextCtrl, e.g. position 0 in the empty control, so account for it and, notably, don't return -1 from XYToPosition(0, 0) when the control is empty. This fixes a regression in a69ab2907c24ac60af8512ca357124a7caa027a2. --- src/msw/textctrl.cpp | 11 +++++------ tests/controls/textctrltest.cpp | 16 ++++++++-------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/msw/textctrl.cpp b/src/msw/textctrl.cpp index 34daff6cce..842a08b364 100644 --- a/src/msw/textctrl.cpp +++ b/src/msw/textctrl.cpp @@ -1486,13 +1486,12 @@ long wxTextCtrl::XYToPosition(long x, long y) const // Line is identified by a character position! long lineLength = ::SendMessage(GetHwnd(), EM_LINELENGTH, charIndex, 0); - // For all lines but last one we need to adjust the length - // to include new line character (only one because both CR and LF - // are virtually "displayed" at the same position). - if ( y < GetNumberOfLines() - 1 ) - lineLength += 1; - if ( x >= lineLength ) + // Notice that x == lineLength is still valid because it corresponds either + // to the position of the LF at the end of any line except the last one or + // to the last position, which is the position after the last character, + // for the last line. + if ( x > lineLength ) return -1; return charIndex + x; diff --git a/tests/controls/textctrltest.cpp b/tests/controls/textctrltest.cpp index 15e2c0d3e2..954acfa61f 100644 --- a/tests/controls/textctrltest.cpp +++ b/tests/controls/textctrltest.cpp @@ -831,7 +831,7 @@ void TextCtrlTestCase::XYToPositionMultiLine() const long numLines_0 = 1; CPPUNIT_ASSERT_EQUAL( m_text->GetNumberOfLines(), numLines_0 ); long pos_0[numLines_0+1][maxLineLength_0+1] = - { { -1 } }; + { { 0 } }; for ( long y = 0; y < numLines_0; y++ ) for( long x = 0; x < maxLineLength_0+1; x++ ) { @@ -846,7 +846,7 @@ void TextCtrlTestCase::XYToPositionMultiLine() const long numLines_1 = 1; CPPUNIT_ASSERT_EQUAL( m_text->GetNumberOfLines(), numLines_1 ); long pos_1[numLines_1+1][maxLineLength_1+1] = - { { 0, 1, 2, 3, -1 }, + { { 0, 1, 2, 3, 4 }, { -1, -1, -1, -1, -1 } }; for ( long y = 0; y < numLines_1; y++ ) for( long x = 0; x < maxLineLength_1+1; x++ ) @@ -866,7 +866,7 @@ void TextCtrlTestCase::XYToPositionMultiLine() long pos_2[numLines_2 + 1][maxLineLength_2 + 1] = { { 0, 1, 2, 3, -1 }, // New line occupies positions 3, 4 { 5, 6, 7, -1, -1 }, // New line occupies positions 7, 8 - { 9, -1, -1, -1, -1 } }; + { 9, 10, -1, -1, -1 } }; #else long pos_2[numLines_2+1][maxLineLength_2+1] = { { 0, 1, 2, 3, -1 }, @@ -893,7 +893,7 @@ void TextCtrlTestCase::XYToPositionMultiLine() { { 0, -1 }, // New line occupies positions 0, 1 { 2, -1 }, // New line occupies positions 2, 3 { 4, -1 }, // New line occupies positions 4, 5 - { -1, -1 }, + { 6, -1 }, { -1, -1 } }; #else long pos_3[numLines_3+1][maxLineLength_3+1] = @@ -925,7 +925,7 @@ void TextCtrlTestCase::XYToPositionMultiLine() { 8, -1, -1, -1, -1 }, // New line occupies positions 8, 9 { 10, 11, -1, -1, -1 }, // New line occupies positions 11, 12 { 13, -1, -1, -1, -1 }, // New line occupies positions 13, 14 - { -1, -1, -1, -1, -1 }, + { 15, -1, -1, -1, -1 }, { -1, -1, -1, -1, -1 } }; #else long pos_4[numLines_4+1][maxLineLength_4+1] = @@ -1013,7 +1013,7 @@ void TextCtrlTestCase::XYToPositionSingleLine() for( long x = 0; x < m_text->GetLastPosition()+2; x++ ) { long p0 = m_text->XYToPosition(x, 0); - if ( x < m_text->GetLastPosition() ) + if ( x <= m_text->GetLastPosition() ) CPPUNIT_ASSERT_EQUAL( p0, x ); else CPPUNIT_ASSERT_EQUAL( p0, -1 ); @@ -1029,7 +1029,7 @@ void TextCtrlTestCase::XYToPositionSingleLine() for( long x = 0; x < m_text->GetLastPosition()+2; x++ ) { long p1 = m_text->XYToPosition(x, 0); - if ( x < m_text->GetLastPosition() ) + if ( x <= m_text->GetLastPosition() ) CPPUNIT_ASSERT_EQUAL( p1, x ); else CPPUNIT_ASSERT_EQUAL( p1, -1 ); @@ -1045,7 +1045,7 @@ void TextCtrlTestCase::XYToPositionSingleLine() for( long x = 0; x < m_text->GetLastPosition()+2; x++ ) { long p2 = m_text->XYToPosition(x, 0); - if ( x < m_text->GetLastPosition() ) + if ( x <= m_text->GetLastPosition() ) CPPUNIT_ASSERT_EQUAL( p2, x ); else CPPUNIT_ASSERT_EQUAL( p2, -1 ); From 49ebac7102851d2c9d0e8e29404fda43b679383b Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 25 Aug 2017 01:12:17 +0200 Subject: [PATCH 2/3] Revert "Simplify wxTextCtrl::GetLastPosition" This reverts commit c4e1fb4ef939a8297eb017f89df06f5f6f72c539. Last position is not necessarily the number of characters in the buffer, rich edit controls still store the text using CR LF between lines, but GetLastPosition() should return the position as if they used only LF to be consistent with all the other positions in these controls and changing this broke existing code passing GetLastPosition() to other functions taking position. --- src/msw/textctrl.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/msw/textctrl.cpp b/src/msw/textctrl.cpp index 842a08b364..028d9709c0 100644 --- a/src/msw/textctrl.cpp +++ b/src/msw/textctrl.cpp @@ -1334,7 +1334,12 @@ wxTextPos wxTextCtrl::GetLastPosition() const { if ( IsMultiLine() ) { - return ::GetWindowTextLength(GetHwnd()); + int numLines = GetNumberOfLines(); + long posStartLastLine = XYToPosition(0, numLines - 1); + + long lenLastLine = GetLengthOfLineContainingPos(posStartLastLine); + + return posStartLastLine + lenLastLine; } return wxTextEntry::GetLastPosition(); From af0a938a65b20a171fea6a1bfc5bb4840883d067 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 25 Aug 2017 01:34:04 +0200 Subject: [PATCH 3/3] Enable InsertionPoint unit test for multiline text controls too There doesn't seem to be any reason to not run it. --- tests/controls/textctrltest.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/controls/textctrltest.cpp b/tests/controls/textctrltest.cpp index 954acfa61f..6d3579077c 100644 --- a/tests/controls/textctrltest.cpp +++ b/tests/controls/textctrltest.cpp @@ -71,6 +71,7 @@ private: // don't pass neither but this could be a bug. CPPUNIT_TEST( SetValue ); CPPUNIT_TEST( Selection ); + CPPUNIT_TEST( InsertionPoint ); CPPUNIT_TEST( Replace ); WXUISIM_TEST( Editable ); CPPUNIT_TEST( CopyPaste );