Fix logic in wxTextWrapper::Wrap()

The existing code behaved completely wrongly whenever a word longer than
the maximal width was present as it just gave up and didn't wrap
anything at all after it -- when it should have just let this word
overflow but still wrap the rest.

Fix this and add a unit test checking that this works correctly now
(several of the tests would have failed before).

See #23339, #25100.

(cherry picked from commit 5972730200)
This commit is contained in:
Vadim Zeitlin
2025-01-31 19:34:20 +01:00
parent 89ba8f190b
commit 3482c83535
9 changed files with 181 additions and 7 deletions
+1
View File
@@ -272,6 +272,7 @@ All (GUI):
- Improve wxPropGrid checkboxes in high DPI (Alex Shvartzkop, #24650, #24651).
- Don't break AUI layout if Update() called when window is iconized (#24930).
- Improve wxInfoBar appearance (#24838, #24902, #25048).
- Fix multiple bugs in wxStaticText::Wrap() (#23339).
- Fix WX_GL_COMPAT_PROFILE spelling (#24964).
wxGTK:
+15 -7
View File
@@ -142,19 +142,27 @@ void wxTextWrapper::Wrap(wxWindow *win, const wxString& text, int widthMax)
}
// Find the last word to chop off.
const size_t lastSpace = line.rfind(' ', posEnd);
if ( lastSpace == wxString::npos )
size_t posSpace = line.rfind(' ', posEnd);
if ( posSpace == wxString::npos )
{
// No spaces, so can't wrap.
DoOutputLine(line);
break;
// No spaces, so can't wrap, output until the end of the word
// which is defined here as just a sequence of non-space chars.
//
// TODO: Implement real Unicode word break algorithm.
posSpace = line.find(' ', posEnd);
if ( posSpace == wxString::npos )
{
// No more spaces at all, output the rest of the line.
DoOutputLine(line);
break;
}
}
// Output the part that fits.
DoOutputLine(line.substr(0, lastSpace));
DoOutputLine(line.substr(0, posSpace));
// And redo the layout with the rest.
line = line.substr(lastSpace + 1);
line = line.substr(posSpace + 1);
}
}
}
+4
View File
@@ -265,6 +265,7 @@ TEST_GUI_OBJECTS = \
test_gui_garbage.o \
test_gui_safearrayconverttest.o \
test_gui_settings.o \
test_gui_textwrap.o \
test_gui_socket.o \
test_gui_tlw.o \
test_gui_dataview.o \
@@ -1175,6 +1176,9 @@ test_gui_safearrayconverttest.o: $(srcdir)/misc/safearrayconverttest.cpp $(TEST_
test_gui_settings.o: $(srcdir)/misc/settings.cpp $(TEST_GUI_ODEP)
$(CXXC) -c -o $@ $(TEST_GUI_CXXFLAGS) $(srcdir)/misc/settings.cpp
test_gui_textwrap.o: $(srcdir)/misc/textwrap.cpp $(TEST_GUI_ODEP)
$(CXXC) -c -o $@ $(TEST_GUI_CXXFLAGS) $(srcdir)/misc/textwrap.cpp
test_gui_socket.o: $(srcdir)/net/socket.cpp $(TEST_GUI_ODEP)
$(CXXC) -c -o $@ $(TEST_GUI_CXXFLAGS) $(srcdir)/net/socket.cpp
+4
View File
@@ -239,6 +239,7 @@ TEST_GUI_OBJECTS = \
$(OBJS)\test_gui_garbage.o \
$(OBJS)\test_gui_safearrayconverttest.o \
$(OBJS)\test_gui_settings.o \
$(OBJS)\test_gui_textwrap.o \
$(OBJS)\test_gui_socket.o \
$(OBJS)\test_gui_tlw.o \
$(OBJS)\test_gui_dataview.o \
@@ -1134,6 +1135,9 @@ $(OBJS)\test_gui_safearrayconverttest.o: ./misc/safearrayconverttest.cpp
$(OBJS)\test_gui_settings.o: ./misc/settings.cpp
$(CXX) -c -o $@ $(TEST_GUI_CXXFLAGS) $(CPPDEPS) $<
$(OBJS)\test_gui_textwrap.o: ./misc/textwrap.cpp
$(CXX) -c -o $@ $(TEST_GUI_CXXFLAGS) $(CPPDEPS) $<
$(OBJS)\test_gui_socket.o: ./net/socket.cpp
$(CXX) -c -o $@ $(TEST_GUI_CXXFLAGS) $(CPPDEPS) $<
+4
View File
@@ -255,6 +255,7 @@ TEST_GUI_OBJECTS = \
$(OBJS)\test_gui_garbage.obj \
$(OBJS)\test_gui_safearrayconverttest.obj \
$(OBJS)\test_gui_settings.obj \
$(OBJS)\test_gui_textwrap.obj \
$(OBJS)\test_gui_socket.obj \
$(OBJS)\test_gui_tlw.obj \
$(OBJS)\test_gui_dataview.obj \
@@ -1432,6 +1433,9 @@ $(OBJS)\test_gui_safearrayconverttest.obj: .\misc\safearrayconverttest.cpp
$(OBJS)\test_gui_settings.obj: .\misc\settings.cpp
$(CXX) /c /nologo /TP /Fo$@ $(TEST_GUI_CXXFLAGS) .\misc\settings.cpp
$(OBJS)\test_gui_textwrap.obj: .\misc\textwrap.cpp
$(CXX) /c /nologo /TP /Fo$@ $(TEST_GUI_CXXFLAGS) .\misc\textwrap.cpp
$(OBJS)\test_gui_socket.obj: .\net\socket.cpp
$(CXX) /c /nologo /TP /Fo$@ $(TEST_GUI_CXXFLAGS) .\net\socket.cpp
+148
View File
@@ -0,0 +1,148 @@
///////////////////////////////////////////////////////////////////////////////
// Name: tests/misc/textwrap.cpp
// Purpose: wxTextWrapper unit test
// Author: Vadim Zeitlin
// Created: 2025-01-19
// Copyright: (c) 2025 Vadim Zeitlin
///////////////////////////////////////////////////////////////////////////////
// ----------------------------------------------------------------------------
// headers
// ----------------------------------------------------------------------------
#include "testprec.h"
#ifndef WX_PRECOMP
#include "wx/app.h"
#endif // !PCH
#include "wx/textwrapper.h"
// ----------------------------------------------------------------------------
// wrapper implementation for testing
// ----------------------------------------------------------------------------
namespace
{
class HardBreakWrapper : public wxTextWrapper
{
public:
HardBreakWrapper() = default;
// Return the window used for wrapping: for now, use the main frame because
// it doesn't really matter which one we use.
wxWindow* GetWindow() const
{
return wxTheApp->GetTopWindow();
}
// Helper function returning the width of the given text.
int GetExtent(const wxString& text) const
{
return GetWindow()->GetTextExtent(text).x;
}
// Wrap and return the number of lines in the wrapped text.
size_t Do(const wxString& text, int width)
{
Wrap(GetWindow(), text, width);
return m_lines.size();
}
const wxString& GetLine(size_t n) const
{
REQUIRE( n < m_lines.size() );
return m_lines[n];
}
wxString GetResult() const { return wxJoin(m_lines, '\n'); }
protected:
void OnOutputLine(const wxString& line) override { m_lines.push_back(line); }
private:
wxArrayString m_lines;
};
} // anonymous namespace
// ----------------------------------------------------------------------------
// the tests
// ----------------------------------------------------------------------------
TEST_CASE("wxTextWrapper::Wrap", "[text]")
{
// Note that this text shouldn't contain line breaks.
const wxString text =
"Lorem ipsum dolor sit amet, consectetur adipiscing elit, "
"sed do eiusmod tempor incididunt ut labore et dolore magna aliqua."
;
HardBreakWrapper w;
// Check that not wrapping the text works.
SECTION("None")
{
REQUIRE( w.Do(text, -1) == 1 );
CHECK( w.GetLine(0) == text );
}
SECTION("Wide")
{
REQUIRE( w.Do(text, 200*w.GetExtent("x")) == 1 );
CHECK( w.GetLine(0) == text );
}
// Check that wrapping the text using reasonable width works.
SECTION("Normal")
{
const auto n = w.Do(text, 40*w.GetExtent("x"));
INFO("Wrapped text:\n" << w.GetResult() << "\n");
CHECK( n >= 3 );
}
SECTION("Narrow")
{
const auto n = w.Do(text, 20*w.GetExtent("x"));
INFO("Wrapped text:\n" << w.GetResult() << "\n");
CHECK( n >= 7 );
}
// Make the window too narrow to fit the word "consectetur" and check that
// the text is still wrapped reasonableness well.
SECTION("Thin")
{
const auto n = w.Do(text, w.GetExtent("Lorum"));
INFO("Wrapped text:\n" << w.GetResult() << "\n");
CHECK( n > 10 );
CHECK( w.GetLine(0) == "Lorem" );
// There should be just one line with the word "consectetur", longer
// than the wrap width.
bool found = false;
for ( size_t i = 1; i < n; ++i )
{
if ( w.GetLine(i) == "consectetur" )
{
found = true;
break;
}
}
CHECK( found );
}
// Make the window too narrow to fit even a single character so that
// wrapped text has one word per line.
SECTION("Narrowest")
{
const auto n = w.Do(text, 1);
INFO("Wrapped text:\n" << w.GetResult() << "\n");
REQUIRE( n == static_cast<size_t>(text.Freq(' ')) + 1 );
CHECK( w.GetLine(0) == "Lorem" );
CHECK( w.GetLine(5) == "consectetur" );
}
}
+1
View File
@@ -269,6 +269,7 @@
misc/garbage.cpp
misc/safearrayconverttest.cpp
misc/settings.cpp
misc/textwrap.cpp
<!--
This one is intentionally duplicated here (it is also part of
non-GUI test) as sockets behave differently in console and GUI
+1
View File
@@ -568,6 +568,7 @@
<ClCompile Include="misc\safearrayconverttest.cpp" />
<ClCompile Include="misc\selstoretest.cpp" />
<ClCompile Include="misc\settings.cpp" />
<ClCompile Include="misc\textwrap.cpp" />
<ClCompile Include="net\socket.cpp" />
<ClCompile Include="persistence\tlw.cpp" />
<ClCompile Include="persistence\dataview.cpp" />
+3
View File
@@ -257,6 +257,9 @@
<ClCompile Include="controls\textentrytest.cpp">
<Filter>Source Files</Filter>
</ClCompile>
<ClCompile Include="misc\textwrap.cpp">
<Filter>Source Files</Filter>
</ClCompile>
<ClCompile Include="controls\togglebuttontest.cpp">
<Filter>Source Files</Filter>
</ClCompile>