`wxFileDirPickerCtrlBase`: Initial path exceeding 32 characters invisible (Issue #26314)
Denis Barucic
barucden created an issue (wxWidgets/wxWidgets#26314)Description
Passing an initial path exceeding 32 characters to the constructor of wxFilePickerCtrl (with wxFLP_USE_TEXTCTRL) results in an empty text box on wxGTK.
Expected behavior
The initial path is visible in the text box.
Observed behavior
The text box is empty and the initial path is not displayed. The path can be made visible by clicking inside the text box and clicking outside again.
Steps to reproduce
Create a CMake project with the following CMakeLists.txt:
cmake_minimum_required(VERSION 3.21) project(Repro) include(FetchContent) FetchContent_Declare( wxWidgets GIT_REPOSITORY https://github.com/wxWidgets/wxWidgets.git GIT_TAG v3.3.2 GIT_SHALLOW ON ) FetchContent_MakeAvailable(wxWidgets) add_executable(repro main.cpp) target_link_libraries(repro PRIVATE wx::core wx::base)
Create main.cpp:
#include <wx/app.h> #include <wx/filepicker.h> #include <wx/frame.h> #include <wx/panel.h> #include <wx/sizer.h> class MainFrame : public wxFrame { public: MainFrame() : wxFrame(nullptr, wxID_ANY, "Repro") { wxPanel *panel = new wxPanel(this); // 32 characters wxString str32 = "12345678901234567890123456789012"; // 33 characters wxString str33 = "123456789012345678901234567890123"; // File picker with 32-character initial path auto *fp32 = new wxFilePickerCtrl(panel, wxID_ANY, str32, wxFileSelectorPromptStr, wxFileSelectorDefaultWildcardStr, wxDefaultPosition, wxDefaultSize, wxFLP_USE_TEXTCTRL); // File picker with 33-character initial path auto *fp33 = new wxFilePickerCtrl(panel, wxID_ANY, str33, wxFileSelectorPromptStr, wxFileSelectorDefaultWildcardStr, wxDefaultPosition, wxDefaultSize, wxFLP_USE_TEXTCTRL); // File picker with empty initial path // 33-character path is set after construction using SetPath auto *fp33_SetPath = new wxFilePickerCtrl(panel, wxID_ANY, wxEmptyString, wxFileSelectorPromptStr, wxFileSelectorDefaultWildcardStr, wxDefaultPosition, wxDefaultSize, wxFLP_USE_TEXTCTRL); fp33_SetPath->SetPath(str33); auto* sizer = new wxBoxSizer(wxVERTICAL); sizer->Add(fp32, 0, wxEXPAND); sizer->Add(fp33, 0, wxEXPAND); sizer->Add(fp33_SetPath, 0, wxEXPAND); panel->SetSizerAndFit(sizer); } }; class MyApp : public wxApp { public: bool OnInit() override { MainFrame* frame = new MainFrame(); frame->Show(true); return true; } }; wxDECLARE_APP(MyApp); wxIMPLEMENT_APP(MyApp);
You will see three filepickers:
- The first filepicker is initialized with a "path" that is exactly 32 characters long. This path is correctly displayed in the text box.
- The second filepicker is initialized with a path that is 33 characters long. This path is not visible in the text box until the user clicks inside & outside the text box.
- The second filepicker is initialized with an empty path. A path of 33 characters is set using
SetPathafter the construction. This path is correctly displayed in the text box.
I recorded my screen here:
Note that wxDirPickerCtrl behaves the same.
Suggested fix
I have a rough idea about the cause, but I don't know the codebase enough to be absolutely certain.
I believe this is what's happening:
- The initial path is passed all the way to
wxPickerBase::CreateBase. - This method, however, sets the maximum length of the
wxTextCtrlto 32 and sets the initial path as the value of the text box:
https://github.com/wxWidgets/wxWidgets/blob/d05c5c1d3e55542e63711778193175c099774f60/src/common/pickerbase.cpp#L85-L88 - The maximum length is increased later:
https://github.com/wxWidgets/wxWidgets/blob/d05c5c1d3e55542e63711778193175c099774f60/src/common/filepickercmn.cpp#L89-L91
But the value does not show up until further interaction with the widget.
I managed to fix the behavior by changing the code wxFileDirPickerCtrlBase::CreateBase shown above like this:
- if (m_text) m_text->SetMaxLength(512); + if (m_text) { + m_text->SetMaxLength(512); + m_text->ChangeValue(path); + }
It is probably not the best solution though.
Platform and version information
- wxWidgets version you use: Spotted using 3.3.1, reproduced using 3.3.2
- wxWidgets port you use: wxGTK
- OS and its version: Archlinux
- GTK version: 3.24.51
- Which GDK backend is used: X11
- Desktop environment : i3wm
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
paulcor
paulcor left a comment (wxWidgets/wxWidgets#26314)This is enough to avoid the problem:
diff --git a/src/common/pickerbase.cpp b/src/common/pickerbase.cpp index 647c6e5baf..ffe78ecee0 100644 --- a/src/common/pickerbase.cpp +++ b/src/common/pickerbase.cpp @@ -82,7 +82,7 @@ bool wxPickerBase::CreateBase(wxWindow *parent, // the m_picker; for very long strings, this real-time synchronization could // become a CPU-blocker and thus should be avoided. // 32 characters will be more than enough for all common uses. - m_text->SetMaxLength(32); + m_text->SetMaxLength(wxMax(32, text.length())); // set the initial contents of the textctrl m_text->SetValue(text);
It's unclear from the preceding comment whether this max length was actually "very important", or whether it still is 20 years later...
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26314)I agree that we should just remove this call to SetMaxLength(), however there is another problem here: max length is not supposed to affect calls to SetValue(), and I'm almost sure it doesn't in wxMSW, so it looks like there is an inconsistency between platforms here.
Anyhow, I'll make a PR to just remove the call here because I really don't know what is it supposed to achieve. Please let me know if I'm missing anything.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Denis Barucic
barucden left a comment (wxWidgets/wxWidgets#26314)Correct, I cannot replicate this on Windows.
It is probably clear to you, but this also replicates the behavior:
auto *textbox32 = new wxTextCtrl(panel, wxID_ANY, wxEmptyString); textbox32->SetMaxLength(32); textbox32->SetValue(str32); // Text visible auto *textbox33 = new wxTextCtrl(panel, wxID_ANY, wxEmptyString); textbox33->SetMaxLength(32); textbox33->SetValue(str33); // Text invisible
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Denis Barucic
barucden left a comment (wxWidgets/wxWidgets#26314)I don't understand the logic, but the execution hits this path:
https://github.com/wxWidgets/wxWidgets/blob/da02b000f4099bef7af084243d96eea2736546ca/src/gtk/textentry.cpp#L186-L189
text_length is 0 here and g_utf8_strlen returns 33, while text_max_length is 32 . Consequently, handled is set to true (note the comment above that line), which results in this being executed:
https://github.com/wxWidgets/wxWidgets/blob/da02b000f4099bef7af084243d96eea2736546ca/src/gtk/textentry.cpp#L240-L247
and my understanding is that g_signal_stop_emission_by_name will stop the text insertion.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26314)I've looked into this and (re)discovered that it isn't possible to allow inserting more than the max length characters into a GtkEntry even programmatically because gtk_entry_set_max_length() doesn't allow having more than that many characters — and temporarily setting the max length to 0, inserting the text and then setting it to the old value doesn't help because it just truncates the text.
So I'll just merge the linked PR to fix this bug but leave the rest as is, even if it is unfortunate that the behaviour is not consistent between platforms.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Denis Barucic
barucden left a comment (wxWidgets/wxWidgets#26314)Thank you for your work!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.