wxWidgets / wxWidgets

Cross-Platform C++ GUI Library
https://www.wxwidgets.org/
6.06k stars 1.76k forks source link

Empty button/checkbox label become default "button" as text... #17152

Closed wxtrac closed 8 years ago

wxtrac commented 9 years ago

Issue migrated from trac ticket # 17152

component: wxOSX | priority: high | resolution: fixed | keywords: button label regression

2015-09-17 15:49:59: rocrail (Rob Versluis) created the issue


Hi,

in the head revision of 3.1 I encounter under Mac OS X unwanted button/checkbox labels. For example the help button in the Standard Button Sizes has just a question mark in its round shape but with the head revision of 3.1 its overwritten with the text "button". This also happens with checkboxes with empty labels.

wxtrac commented 9 years ago

2015-09-17 15:52:51: rocrail (Rob Versluis) uploaded file stdbutton-help.png (11.4 KiB)

The standard button help. stdbutton-help.png

wxtrac commented 9 years ago

2015-09-17 15:53:21: rocrail (Rob Versluis) uploaded file checkbox-without-label-text.png (13.9 KiB)

Checkboxes without label text are replaced with "button" checkbox-without-label-text.png

wxtrac commented 9 years ago

2015-09-17 16:48:16: @vadz commented


Can this be seen in any of the samples too?

wxtrac commented 9 years ago

2015-09-17 16:51:25: rocrail (Rob Versluis) commented


I just made a new 'git pull' from the 3.1 branch and recompiled everything.

wxtrac commented 9 years ago

2015-09-18 11:23:28: rocrail (Rob Versluis) uploaded file minimal.cpp (6.8 KiB)

A button with empty text/label is added to the minimal.cpp.

wxtrac commented 9 years ago

2015-09-18 11:24:42: rocrail (Rob Versluis) commented


I added a wxButton in the minimal sample with an empty text/label. The Text in the Button will be "Button".... See attachment with the minimal.cpp file.

Starting at line 175:

    // begin ticket # 17152: Default button text...
    SetSizer(new wxBoxSizer(wxHORIZONTAL));
    wxButton* l_Button1 = new wxButton(this, 98, wxT(""));
    // end   ticket # 17152
wxtrac commented 9 years ago

2015-09-18 11:31:11: rocrail (Rob Versluis) uploaded file minimal-buttontext.png (32.5 KiB)

Screen shot of the minimal sample with one button and empty text. minimal-buttontext.png

wxtrac commented 9 years ago

2015-09-18 11:39:26: rocrail (Rob Versluis) commented


Maybe the Mac OSX SDK 10.11 and/or the Xcode 7 command line tools are involved in this issue.

wxtrac commented 9 years ago

2015-09-20 08:11:45: rocrail (Rob Versluis) changed priority from normal to high

wxtrac commented 9 years ago

2015-09-21 09:07:41: @bigjohnr commented


This is a result of "Only set native label if nonempty in wxWindowMac::SetPeer()" https://github.com/wxWidgets/wxWidgets/commit/db9baf9aa5565a7a4f92097b2f0c85439c35b4a6

To work around comment line 359 if ( !m_label.empty() ) in src/osxs/window_osx.cpp

wxBU_NOTEXT doesn't work also.

wxtrac commented 8 years ago

2015-10-17 18:20:21: @vadz changed status from new to accepted

2015-10-17 18:20:21: @vadz commented

Sorry for the delay, could you please test the change at http://review.wxwidgets.org/r/615/ and let me know if you still have any problems with it?

wxtrac commented 8 years ago

2015-10-18 03:04:27: @bigjohnr commented


It works for a button created with an empty label. It won't work for wxButton::SetLabel with an empty string.

We also need some logic for wxBU_NOTEXT similar to wxAnyButton::SetLabel before SetPeer(wxWidgetImpl::CreateButton. I guess that is another ticket...

wxtrac commented 8 years ago

2015-10-18 03:05:43: @vadz commented


Sorry, could you please explain what exactly won't work? I just don't see it...

wxtrac commented 8 years ago

2015-10-18 03:49:39: @bigjohnr commented


Probably not a common usage but: butt* = new wxButton( this, wxID_ANY, "label",...

Later use butt->SetLabel(""); won't set the label to an empty string

With reference to the pre-existing wxBU_NOTEXT problem: butt* = new wxButton( panelCase, wxID_ANY, "label",...,wxBU_NOTEXT ) will still show a label. This is unchanged from previously.

wxtrac commented 8 years ago

2015-10-18 03:54:56: @vadz commented


Sorry, I still don't understand, it works just fine for me with the following test code:

#!diff
diff --git a/samples/minimal/minimal.cpp b/samples/minimal/minimal.cpp
index 076d88b..7774c47 100644
--- a/samples/minimal/minimal.cpp
+++ b/samples/minimal/minimal.cpp
@@ -166,6 +166,11 @@ MyFrame::MyFrame(const wxString& title)
     SetMenuBar(menuBar);
 #endif // wxUSE_MENUS

+    new wxButton(this, wxID_ANY, "", wxPoint(5, 5));
+    (new wxButton(this, wxID_ANY, "Press", wxPoint(5, 55)))->SetLabel("");
+
+    new wxCheckBox(this, wxID_ANY, "", wxPoint(105, 5));
+    (new wxCheckBox(this, wxID_ANY, "Check", wxPoint(105, 55)))->SetLabel("");
 #if wxUSE_STATUSBAR
     // create a status bar just for fun (by default with 1 pane only)
     CreateStatusBar(2);

And wxBU_NOTEXT works too...

Could you please show a diff that fails to work with my proposed changes?

wxtrac commented 8 years ago

2015-10-18 04:27:49: @bigjohnr commented


My mistake, I had tested with wxBU_NOTEXT in the style and you can see in wxAnyButton::SetLabel that it will just return without changing the label.

->SetLabel("") does work without wxBU_NOTEXT.

void wxAnyButton::SetLabel(const wxString& label)
{
    if ( HasFlag(wxBU_NOTEXT) )
    {
        // just store the label internally but don't really use it for the
        // button
        m_labelOrig #         m_labellabel;
        return;
    }

    wxAnyButtonBase::SetLabel(label);
}
wxtrac commented 8 years ago

2015-10-18 05:00:06: @bigjohnr commented


diff --git a/samples/minimal/minimal.cpp b/samples/minimal/minimal.cpp
index 6ae9ab8..e6014de 100644
--- a/samples/minimal/minimal.cpp
+++ b/samples/minimal/minimal.cpp
@@ -168,11 +168,11 @@ MyFrame::MyFrame(const wxString& title)
+     //this is good
+     new wxButton(this, wxID_ANY, "", wxPoint(5, 5));
+     //this is good
+    (new wxButton(this, wxID_ANY, "No label", wxPoint(5, 35)))->SetLabel("");
+     //no good
+    new wxButton(this, wxID_ANY, "No label", wxPoint(5, 55), wxDefaultSize, wxBU_NOTEXT);
+     //no good
+    (new wxButton(this, wxID_ANY, "No label", wxPoint(5, 95), wxDefaultSize, wxBU_NOTEXT))->SetLabel("");
wxtrac commented 8 years ago

2015-10-22 00:08:58: @vadz changed status from accepted to closed

2015-10-22 00:08:58: @vadz changed resolution from * to fixed*

2015-10-22 00:08:58: @vadz commented

In ef1db7acda601ec5dd6240094233938341fbd43e: Create native buttons without any label in wxOSX/Cocoa

Since the changes of db9baf9aa5565a7a4f92097b2f0c85439c35b4a6 the label wasn't explicitly reset to be empty on wxWindow creation because it was assumed it would already be empty if not explicitly set, but this turned out to be false for the controls using NSButton which (very helpfully) uses "Button" as its label by default and so kept this useless label if it wasn't explicitly overridden.

Fix this by explicitly resetting the NSButton title after creating it, to ensure consistency between the real state of the control and what wxWidgets thinks it is.

Closes #17152.

wxtrac commented 8 years ago

2015-10-22 00:12:23: @vadz commented


Replying to [comment:13 johnr]:

//no good new wxButton(this, wxID_ANY, "No label", wxPoint(5, 55), wxDefaultSize, wxBU_NOTEXT);

Sorry, this seems to work just fine for me. Could you please retry with the latest master just to check that we're testing the same code? Please reopen if the problem still persists. TIA!

wxtrac commented 8 years ago

2015-10-22 08:42:16: @bigjohnr commented


Replying to [comment:15 vadz]:

I cloned a fresh copy using git clone https://github.com/wxWidgets/wxWidgets.git and still get the same result.

Perhaps we need to clarify what wxBU_NOTEXT does. I expected to see a button with no label but I get a visible label with the above code and with the code below i.e when I add a bitmap I still see a label. wxMSW shows no label with style wxBU_NOTEXT even when a label is provided.

It is a redundant style in that you can just use an empty string to achieve the same result except in the scenario where you use an empty string and a standard identifier/id.

#include "wx/artprov.h"
    wxButton* butt = new wxButton(this, wxID_CLOSE, "No label", wxPoint(5, 55), wxDefaultSize, wxBU_NOTEXT);
    butt->SetBitmap( wxArtProvider::GetBitmap( wxART_ERROR, wxART_BUTTON ));
wxtrac commented 8 years ago

2015-11-09 03:59:26: @vadz changed status from closed to reopened

2015-11-09 03:59:26: @vadz changed resolution from fixed to **

2015-11-09 03:59:26: @vadz commented

I was wrong, trying this again I see that this indeed doesn't work, sorry, no idea what did I test the last time. I'll try to fix this, although I'm not sure if this is a regression or if this was always a problem in wxOSX to be honest.

wxtrac commented 8 years ago

2015-12-23 05:02:30: @paulcor changed status from reopened to closed

2015-12-23 05:02:30: @paulcor changed resolution from * to fixed*

2015-12-23 05:02:30: @paulcor commented

In f0f83adecfa0af8b65e606e065911e9467335841: Create native buttons without any label in wxOSX/Cocoa

Since the changes of db9baf9aa5565a7a4f92097b2f0c85439c35b4a6 the label wasn't explicitly reset to be empty on wxWindow creation because it was assumed it would already be empty if not explicitly set, but this turned out to be false for the controls using NSButton which (very helpfully) uses "Button" as its label by default and so kept this useless label if it wasn't explicitly overridden.

Fix this by explicitly resetting the NSButton title after creating it, to ensure consistency between the real state of the control and what wxWidgets thinks it is.

Closes #17152.

(cherry picked from commit ef1db7acda601ec5dd6240094233938341fbd43e)

wxtrac commented 8 years ago

2015-12-23 05:10:59: @paulcor changed status from closed to reopened

2015-12-23 05:10:59: @paulcor changed resolution from fixed to **

wxtrac commented 8 years ago

2016-02-01 03:14:21: @vadz changed resolution from * to fixed*

2016-02-01 03:14:21: @vadz changed status from reopened to closed

2016-02-01 03:14:21: @vadz commented

In 9b39ffc0cbee406caacf3806d2817ce8c751cb23: Ignore initially specified labels for buttons with wxBU_NOTEXT

It doesn't make much sense to specify a non-empty label and wxBU_NOTEXT style together, but if this happens, the label should be ignored, as it was already done by wxGTK, but not wxMSW and wxOSX -- so add the missing checks for wxBU_NOTEXT to these ports too.

Closes #17152.