Merge branch 'mac-textctrl-size-refactor'

Don't call GetBestSize() from DoGetSizeFromTextSize() in wxOSX.

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

include/wx/osx/cocoa/private/textimpl.h

@@ -32,6 +32,12 @@ public :
     virtual ~wxNSTextBase() { }
 
     virtual bool ShouldHandleKeyNavigation(const wxKeyEvent &event) const wxOVERRIDE;
+
+    virtual void SetInitialLabel(const wxString& WXUNUSED(title), wxFontEncoding WXUNUSED(encoding)) wxOVERRIDE
+    {
+        // Don't do anything here, text controls don't have any label and
+        // setting it would overwrite the string value set when creating it.
+    }
 };
 
 // implementation exposed, so that search control can pull it

src/osx/cocoa/textctrl.mm

@@ -1222,13 +1222,25 @@ void wxNSTextViewControl::EnableAutomaticDashSubstitution(bool enable)
 
 wxSize wxNSTextViewControl::GetBestSize() const
 {
-    if (m_textView && [m_textView layoutManager])
+    wxSize size;
+    if ( NSLayoutManager* const layoutManager = [m_textView layoutManager] )
     {
-        NSRect rect = [[m_textView layoutManager] usedRectForTextContainer: [m_textView textContainer]];
+        // We could have used usedRectForTextContainer: here to compute the
-        return wxSize((int)(rect.size.width + [m_textView textContainerInset].width),
+        // total rectangle needed for the current text, but this is actually
-                      (int)(rect.size.height + [m_textView textContainerInset].height));
+        // not too helpful for determining the best size of the control, as we
+        // don't always want to fit it to its text: e.g. if it contains 1000
+        // lines, we definitely don't want to make it higher than display.
+        //
+        // So instead we just get the height of a single line (which works for
+        // any non-empty control) and then multiply it by the number of lines
+        // we want to have by default, which is currently just hardcoded as 5
+        // for compatibility with the behaviour of the previous versions.
+        NSRect rect = [layoutManager lineFragmentRectForGlyphAtIndex: 0
+                                     effectiveRange: nil];
+        size.y = (int)(5*rect.size.height + [m_textView textContainerInset].height);
     }
-    return wxSize(0,0);
+
+    return size;
 }
 
 void wxNSTextViewControl::SetJustification()
@@ -1556,7 +1568,7 @@ void wxNSTextFieldControl::SetJustification()
 wxWidgetImplType* wxWidgetImpl::CreateTextControl( wxTextCtrl* wxpeer,
                                     wxWindowMac* WXUNUSED(parent),
                                     wxWindowID WXUNUSED(id),
-                                    const wxString& WXUNUSED(str),
+                                    const wxString& str,
                                     const wxPoint& pos,
                                     const wxSize& size,
                                     long style,
@@ -1569,8 +1581,11 @@ wxWidgetImplType* wxWidgetImpl::CreateTextControl( wxTextCtrl* wxpeer,
     {
         wxNSTextScrollView* v = nil;
         v = [[wxNSTextScrollView alloc] initWithFrame:r];
-        c = new wxNSTextViewControl( wxpeer, v, style );
+        wxNSTextViewControl* t = new wxNSTextViewControl( wxpeer, v, style );
+        c = t;
         c->SetNeedsFocusRect( true );
+
+        t->SetStringValue(str);
     }
     else
     {
@@ -1593,7 +1608,8 @@ wxWidgetImplType* wxWidgetImpl::CreateTextControl( wxTextCtrl* wxpeer,
         [cell setWraps:NO];
         [cell setScrollable:YES];
 
-        c = new wxNSTextFieldControl( wxpeer, wxpeer, v );
+        wxNSTextFieldControl* t = new wxNSTextFieldControl( wxpeer, wxpeer, v );
+        c = t;
 
         if ( (style & wxNO_BORDER) || (style & wxSIMPLE_BORDER) )
         {
@@ -1608,6 +1624,8 @@ wxWidgetImplType* wxWidgetImpl::CreateTextControl( wxTextCtrl* wxpeer,
             // use native border
             c->SetNeedsFrame(false);
         }
+
+        t->SetStringValue(str);
     }
 
     return c;

src/osx/textctrl_osx.cpp

@@ -43,8 +43,6 @@
 
 #include "wx/osx/private.h"
 
-static const int TEXTCTRL_BORDER_SIZE = 5;
-
 wxBEGIN_EVENT_TABLE(wxTextCtrl, wxTextCtrlBase)
     EVT_DROP_FILES(wxTextCtrl::OnDropFiles)
     EVT_CHAR(wxTextCtrl::OnChar)
@@ -105,12 +103,6 @@ bool wxTextCtrl::Create( wxWindow *parent,
 
     MacPostControlCreate(pos, size) ;
 
-#if wxOSX_USE_COCOA
-    // under carbon everything can already be set before the MacPostControlCreate embedding takes place
-    // but under cocoa for single line textfields this only works after everything has been set up
-    GetTextPeer()->SetStringValue(str);
-#endif
-
     // only now the embedding is correct and we can do a positioning update
 
     MacSuperChangedPosition() ;
@@ -186,23 +178,25 @@ bool wxTextCtrl::IsModified() const
 
 wxSize wxTextCtrl::DoGetBestSize() const
 {
-    int wText = -1;
+    wxSize size;
-    int hText = -1;
-
     if (GetTextPeer())
-    {
+        size = GetTextPeer()->GetBestSize();
-        wxSize size = GetTextPeer()->GetBestSize();
-        if (size.x > 0 && size.y > 0)
-        {
-            hText = size.y;
-            wText = size.x;
-        }
-    }
 
-    if ( hText == - 1)
+    // Normally the width passed to GetSizeFromTextSize() is supposed to be
-    {
+    // valid, i.e. positive, but we use our knowledge of its implementation
-        wText = 100 ;
+    // just below to rely on it returning the default size if we don't have
+    // any valid best size in the peer and size remained default-initialized.
+    return GetSizeFromTextSize(size);
+}
 
+wxSize wxTextCtrl::DoGetSizeFromTextSize(int xlen, int ylen) const
+{
+    static const int TEXTCTRL_BORDER_SIZE = 5;
+
+    // Compute the default height if not specified.
+    int hText = ylen;
+    if ( hText <= 0 )
+    {
         // these are the numbers from the HIG:
         switch ( m_windowVariant )
         {
@@ -226,38 +220,26 @@ wxSize wxTextCtrl::DoGetBestSize() const
         // the numbers above include the border size, so subtract it before
         // possibly adding it back below
         hText -= TEXTCTRL_BORDER_SIZE;
-    }
 
-    // as the above numbers have some free space around the text
+        // make the control 5 lines tall by default for consistently with how
-    // we get 5 lines like this anyway
+        // the old code behaved
         if ( m_windowStyle & wxTE_MULTILINE )
             hText *= 5 ;
+    }
+
+    // Keep using the same default 100px width as was used previously in the
+    // special case of having invalid width.
+    wxSize size(xlen > 0 ? xlen : 100, hText);
+
+    // Use extra margin size which works under macOS 10.15: note that we don't
+    // need the vertical margin when using the automatically determined hText.
+    if ( xlen > 0 )
+        size.x += 4;
+    if ( ylen > 0 )
+        size.y += 2;
 
     if ( !HasFlag(wxNO_BORDER) )
-        hText += TEXTCTRL_BORDER_SIZE ;
+        size += wxSize(TEXTCTRL_BORDER_SIZE, TEXTCTRL_BORDER_SIZE) ;
-
-    return wxSize(wText, hText);
-}
-
-wxSize wxTextCtrl::DoGetSizeFromTextSize(int xlen, int ylen) const
-{
-    wxSize size;
-
-    // Initialize to defaults unless both components are specified (at least
-    // one of them should be, but the other one could be -1).
-    if ( xlen <= 0 || ylen <= 0 )
-        size = DoGetBestSize();
-
-    // Use extra margin size which works under macOS 10.15 and also add the
-    // border for consistency with DoGetBestSize() -- we'll remove it below if
-    // it's not needed.
-    if ( xlen > 0 )
-        size.x = xlen + 4 + TEXTCTRL_BORDER_SIZE;
-    if ( ylen > 0 )
-        size.y = ylen + 2 + TEXTCTRL_BORDER_SIZE;
-
-    if ( HasFlag(wxNO_BORDER) )
-        size.DecBy(TEXTCTRL_BORDER_SIZE);
 
     return size;
 }