Merge branch 'ellipsize-fixes'

Fix ellipsization of strings containing mnemonics and TABs and also
improve handling of trailing spaces (by ignoring them).

See https://github.com/wxWidgets/wxWidgets/pull/1649

src/common/ctrlcmn.cpp

@@ -293,33 +293,62 @@ struct EllipsizeCalculator
           m_maxFinalWidthPx(maxFinalWidthPx),
           m_replacementWidthPx(replacementWidthPx)
     {
-        m_isOk = dc.GetPartialTextExtents(s, m_charOffsetsPx);
-        wxASSERT( m_charOffsetsPx.GetCount() == s.length() );
+        size_t expectedOffsetsCount = s.length();
 
+        // Where ampersands are used as mnemonic indicator they should not
+        // affect the overall width of the string and must be removed from the
+        // measurement. Nonetheless, we need to keep them in the string and
+        // have a corresponding entry in m_charOffsetsPx.
         if ( flags & wxELLIPSIZE_FLAGS_PROCESS_MNEMONICS )
         {
-            // The ampersand itself shouldn't count for the width calculation
-            // as it won't be displayed: either it's an ampersand before some
-            // other character in which case it indicates a mnemonic, or it's
-            // an escaped ampersand, in which case only the second one of the
-            // pair of ampersands will be displayed. But we can't just remove
-            // the ampersand as we still need it in the final label, in order
-            // not to lose the mnemonics. Hence we use this hack, and pretend
-            // that ampersands simply have zero width. Of course, it could be
-            // not completely precise, but this is the best we can do without
-            // completely changing this code structure.
+            // Create a copy of the string with the ampersands removed to get
+            // the correct widths.
+            const wxString cpy = wxControl::RemoveMnemonics(s);
+
+            m_isOk = dc.GetPartialTextExtents(cpy, m_charOffsetsPx);
+
+            // Iterate through the original string inserting a cumulative width
+            // value for each ampersand that is the same as the following
+            // character's cumulative width value. Except this is only done
+            // for the first ampersand in a pair (see RemoveMnemonics).
             size_t n = 0;
-            int delta = 0;
+            bool lastWasMnemonic = false;
             for ( wxString::const_iterator it = s.begin();
                   it != s.end();
                   ++it, ++n )
             {
-                if ( *it == '&' )
-                    delta += dc.GetTextExtent(wxS('&')).GetWidth();
-
-                m_charOffsetsPx[n] -= delta;
+                if ( *it == '&' && !lastWasMnemonic )
+                {
+                    if ( (it + 1) != s.end() )
+                    {
+                        int w = m_charOffsetsPx[n];
+                        m_charOffsetsPx.Insert(w, n);
+                        lastWasMnemonic = true;
+                    }
+                    else // Last character is an ampersand.
+                    {
+                        // This ampersand is removed by RemoveMnemonics() and
+                        // won't be displayed when this string is drawn
+                        // neither, so we intentionally don't use it for our
+                        // calculations neither -- just account for this in the
+                        // assert below.
+                        expectedOffsetsCount--;
+                    }
+                }
+                else // Not an ampersand used to introduce a mnemonic.
+                {
+                    lastWasMnemonic = false;
+                }
             }
         }
+        else
+        {
+            m_isOk = dc.GetPartialTextExtents(s, m_charOffsetsPx);
+        }
+
+        // Either way, we should end up with the same number of offsets as
+        // characters in the original string.
+        wxASSERT( m_charOffsetsPx.GetCount() == expectedOffsetsCount );
     }
 
     bool IsOk() const { return m_isOk; }
@@ -567,6 +596,7 @@ wxString wxControlBase::Ellipsize(const wxString& label, const wxDC& dc,
     {
         if ( pc == label.end() || *pc == wxS('\n') )
         {
+            curLine.Trim();
             curLine = DoEllipsizeSingleLine(curLine, dc, mode, maxFinalWidth,
                                             replacementWidth, flags);
 

tests/graphics/ellipsization.cpp

@@ -23,14 +23,7 @@
 // test class
 // ----------------------------------------------------------------------------
 
-class EllipsizationTestCase
-{
-public:
-    EllipsizationTestCase() { }
-};
-
-
-TEST_CASE_METHOD(EllipsizationTestCase, "Ellipsization::NormalCase", "[ellipsization]")
+TEST_CASE("Ellipsization::NormalCase", "[ellipsization]")
 {
     wxMemoryDC dc;
 
@@ -101,8 +94,9 @@ TEST_CASE_METHOD(EllipsizationTestCase, "Ellipsization::NormalCase", "[ellipsiza
                     WX_ASSERT_MESSAGE
                     (
                      (
-                        "Test #(%u,%u.%u): \"%s\" -> \"%s\"; width=%dpx > %dpx",
+                        "Test #(%u,%u.%u): %s\n\"%s\" -> \"%s\"; width=%dpx > %dpx",
                         s, f, m,
+                        dc.GetFont().GetNativeFontInfoUserDesc(),
                         str,
                         ret,
                         width,
@@ -117,7 +111,7 @@ TEST_CASE_METHOD(EllipsizationTestCase, "Ellipsization::NormalCase", "[ellipsiza
 }
 
 
-TEST_CASE_METHOD(EllipsizationTestCase, "Ellipsization::EnoughSpace", "[ellipsization]")
+TEST_CASE("Ellipsization::EnoughSpace", "[ellipsization]")
 {
     // No ellipsization should occur if there's plenty of space.
 
@@ -132,7 +126,7 @@ TEST_CASE_METHOD(EllipsizationTestCase, "Ellipsization::EnoughSpace", "[ellipsiz
 }
 
 
-TEST_CASE_METHOD(EllipsizationTestCase, "Ellipsization::VeryLittleSpace", "[ellipsization]")
+TEST_CASE("Ellipsization::VeryLittleSpace", "[ellipsization]")
 {
     // If there's not enough space, the shortened label should still contain "..." and one character
 
@@ -147,7 +141,7 @@ TEST_CASE_METHOD(EllipsizationTestCase, "Ellipsization::VeryLittleSpace", "[elli
 }
 
 
-TEST_CASE_METHOD(EllipsizationTestCase, "Ellipsization::HasThreeDots", "[ellipsization]")
+TEST_CASE("Ellipsization::HasThreeDots", "[ellipsization]")
 {
     wxMemoryDC dc;