Opened 10 months ago

Last modified 8 months ago

#16003 confirmed defect

The extra control of wxFileDialog doesn't work correctly if it has no wxWindow parent

Reported by: jbbbms Owned by:
Priority: low Milestone:
Component: GUI-all Version: 3.0.0
Keywords: wxFileDialog, SetExtraControlCreator Cc:
Blocked By: Blocking:
Patch: no

Description

The sample program dialogs that comes with wxWidgets works excellent. It uses a sizer to arrange child controls of the panel which is an extra control of the file dialog. What if we only want to add a check box as the extra control? It works fine in wxGTK, but will incur SIGSEGV in wxMSW. What if we don't want to rely on the sizer to reposition and resize the children of the extra control? The extra control won't even show up, whether in wxGTK or wxMSW. The attached file reconstructs the condition. If we must use the sizer when creating the extra control, then it would be nice that the official document reminds us about it. Or, patches are needed to have wxFileDialog::SetExtraControlCreator more adaptable. Thanks. Have a nice day.

Attachments (1)

buggy.cpp download (1.4 KB) - added by jbbbms 10 months ago.
Enable/disable each code paragraph to see the difference.

Download all attachments as: .zip

Change History (7)

Changed 10 months ago by jbbbms

Enable/disable each code paragraph to see the difference.

comment:1 Changed 10 months ago by vadz

  • Priority changed from normal to low
  • Status changed from new to confirmed
  • Summary changed from The extra control of wxFileDialog won't work as expected if it has no sizer. to The extra control of wxFileDialog doesn't work correctly if it has no wxWindow parent

The bug is reproducible with just this (please make such patches if possible instead of the entire programs, even as simple as this one -- because a patch is still simpler -- TIA!):

diff --git a/samples/dialogs/dialogs.cpp b/samples/dialogs/dialogs.cpp
index 331f61c..451cc8b 100644
--- a/samples/dialogs/dialogs.cpp
+++ b/samples/dialogs/dialogs.cpp
@@ -1388,6 +1388,7 @@ MyExtraPanel::MyExtraPanel(wxWindow *parent)
 // a static method can be used instead of a function with most of compilers
 static wxWindow* createMyExtraPanel(wxWindow *parent)
 {
+    return new wxCheckBox(parent, wxID_ANY, "Open with a crash");
     return new MyExtraPanel(parent);
 }

I could fix a crash, but not the fact that the checkbox doesn't actually work inside this dialog. I don't think this has anything to do with the sizers, but it probably is related to not handling the windows messages in the usual way, as they get delivered to the dialog itself instead of some wxWindow. It probably could be fixed too but this is less trivial...

comment:2 Changed 10 months ago by VZ

(In [75937]) Don't use invalid pointer in file dialog hook procedure in wxMSW.

We can receive WM_NOTIFY for other than CDN_XXX messages if we have a native
control as our immediate child (which can happen with "extra" controls) and
the LPARAM is not a pointer to OFNOTIFY at all in this case, so don't try to
use it as such.

This fixes a crash when adding a "bare" extra control, see #16003.

comment:3 Changed 10 months ago by VZ

(In [75941]) Don't use invalid pointer in file dialog hook procedure in wxMSW.

We can receive WM_NOTIFY for other than CDN_XXX messages if we have a native
control as our immediate child (which can happen with "extra" controls) and
the LPARAM is not a pointer to OFNOTIFY at all in this case, so don't try to
use it as such.

This fixes a crash when adding a "bare" extra control, see #16003.

comment:4 Changed 10 months ago by jbbbms

One more thing. If we load the extra control panel from XRC file, it has to be no <size> tag or its child controls will all be positioned at the top-left corner of the panel. This little glitch happens on wxMSW. In contrast, wxGTK doesn't have this problem. For example, modify the sample dialog.cpp, of which the MD5 is f9fe0f3f10dd8c435ac2b49c7d84433b, as the following

--- dialogs.cpp	2013-11-12 14:42:28.000000000 +0800
+++ dialogs.modified.cpp	2014-02-20 15:51:16.000000000 +0800
@@ -1362,7 +1362,9 @@
     wxStaticText *m_label;
 };
 
+#include <wx/xrc/xmlres.h>
 MyExtraPanel::MyExtraPanel(wxWindow *parent)
+#if 0
             : wxPanel(parent)
 {
     m_btn = new wxButton(this, -1, wxT("Custom Button"));
@@ -1383,6 +1385,15 @@
     sizerTop->Add(m_label, wxSizerFlags().Centre().Border());
 
     SetSizerAndFit(sizerTop);
+#else
+            : wxPanel()
+{
+    wxXmlResource * res = new wxXmlResource("noname.xrc");
+    res->InitAllHandlers();
+    res->LoadPanel(this, parent, "MyPanel1");
+    delete res;
+    // For simplicity's sake, the event handlings are not implemented.
+#endif
 }
 
 // a static method can be used instead of a function with most of compilers

and save the following as a XRC file named "noname.xrc" in the same directory as the executable being built.

<?xml version="1.0" encoding="UTF-8" standalone="yes" ?>
<resource xmlns="http://www.wxwindows.org/wxxrc" version="2.3.0.1">
	<object class="wxPanel" name="MyPanel1">
		<style>wxTAB_TRAVERSAL</style>
		<size>389,51</size>
		<object class="wxBoxSizer">
			<orient>wxHORIZONTAL</orient>
			<object class="sizeritem">
				<option>0</option>
				<flag>wxALL|wxALIGN_CENTER_VERTICAL</flag>
				<border>5</border>
				<object class="wxCheckBox" name="-1">
					<label>Enable Custom Button</label>
					<checked>0</checked>
				</object>
			</object>
			<object class="spacer">
				<option>1</option>
				<flag>wxEXPAND|wxALIGN_CENTER_VERTICAL</flag>
				<border>5</border>
				<size>0,0</size>
			</object>
			<object class="sizeritem">
				<option>0</option>
				<flag>wxALL|wxALIGN_CENTER_VERTICAL</flag>
				<border>5</border>
				<object class="wxButton" name="-1">
					<enabled>0</enabled>
					<label>Custom Button</label>
					<default>0</default>
				</object>
			</object>
			<object class="spacer">
				<option>1</option>
				<flag>wxEXPAND|wxALIGN_CENTER_VERTICAL</flag>
				<border>5</border>
				<size>0,0</size>
			</object>
			<object class="sizeritem">
				<option>0</option>
				<flag>wxALL|wxALIGN_CENTER_VERTICAL</flag>
				<border>5</border>
				<object class="wxStaticText" name="wxID_ANY">
					<label>Nothing selected</label>
					<wrap>-1</wrap>
				</object>
			</object>
		</object>
	</object>
</resource>

When the file dialog shows up, we can see the child controls are not at the positions they are supposed to be. This wrong positioning could be circumvented, if we remove the <size> tag of <object class="wxPanel" name="MyPanel1">, or add the following line in the ctor of MyExtraPanel after the invocation of LoadPanel.

    this->GetSizer()->SetSizeHints(this);

It would be nice if the official manual would have a note about the <size> tag.

Thanks. Have a nice day.

comment:5 Changed 10 months ago by vadz

I think the best, albeit not the most efficient, solution to all this would be to make the extra control child of another panel, which we'd always create internally. This shouldn't be very difficult to do but, frankly, I'm not very motivated to do it as it's trivial to do outside of wx...

comment:6 Changed 8 months ago by DS

In 76394:

Fixed wxFileDialog::MSWOnInitDone and wxFileDialog::MSWOnSelChange never being called.

A condition was checking for the wrong CDN_XXX message range resulting in CDN_INITDONE and CDN_SELCHANGE no longer being processed. Broken since r75937 (branches/WX_3_0_BRANCH/) and r75941 (trunk/).

See #16003.

Note: See TracTickets for help on using tickets.