Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#11635 closed defect (fixed)

Selecting a file to save using wxFilePickerCtrl is broken on MacOS

Reported by: swiss_kinkajou Owned by:
Priority: normal Milestone: 2.9.1
Component: wxOSX Version: stable-latest
Keywords: wxFilePickerCtrl Save Cc:
Blocked By: Blocking:
Patch: yes

Description

Using the wxFLP_SAVE style doesn't work as expected. When setting this style the wxFilePickerCtrl doesn't show a save dialog as expected but display an open dialog !

To reproduce:

  • Using the latest trunk (63182) compiled with following options : ../configure --enable-unicode --disable-monolithic --disable-shared --with-osx_cocoa --with-macosx-version-min=10.5
  • Compile the widgets sample.
  • Set the FilePicker control to the save mode
  • Try the browse button

Expected a save dialog
Observed an open dialog

This problem didn't occur in the version 2.8.10

Attachments (1)

filepicker.patch download (1.1 KB) - added by swiss_kinkajou 9 years ago.
wxFilePickerCtrl patch

Download all attachments as: .zip

Change History (8)

Changed 9 years ago by swiss_kinkajou

wxFilePickerCtrl patch

comment:1 Changed 9 years ago by swiss_kinkajou

  • Patch set

Attached a patch allowing wxFilePickerCtrl to display the expected dialog, not only the open dialog.

Problem:
The style specified by user (wxFLP_SAVE or wxFLP_OPEN) wasn't propagated to the child class. Thus the HasFlag() function called in GetDialogStyle() [include/wx/generic/filepickerg.h] was always returning 0.

Just passing the style to the wxButton isn't possible because wxPB_USE_TEXTCTRL value may conflict with the wxBU_NOTEXT hack (same ID values, see [src/osx/button_osx.cpp]).

Solution:
I just removed the wxPB_USE_TEXTCTRL style if existing and passed the style to the child class aka wxButton.

comment:2 Changed 9 years ago by vadz

  • Milestone changed from 2.9.2 to 2.9.1

The problem i definitely serious, thanks for reporting and fixing it! But I just don't understand why does this fix work... Conflict between these two styles values is a problem too, of course, but how is it related to SAVE/OPEN confusion?

Could you please explain?

comment:3 Changed 9 years ago by swiss_kinkajou

I will try to describe a little bit better what I have observed when digging into the wxFilePicker code

When the file picker button is clicked [1], CreateDialog() is called [2]. In this function, a new wxFileDialog is created calling GetDialogStyle() in its constructor. GetDialogStyle is implemented [2] a few lines above and its only purpose is to read the style and set it to the wxFileDialog. So here is the bug : if GetDialogStyle is returning the wrong style, the wrong wxFileDialog will be displayed. This is the case actually because GetDialogStyle return always 0.

Let's dig a little bit deeper into this style problem: GetDialogStyle is calling this->HasFlag(). The value of 'this' is wxGenericFileDirButton (derived from wxButton). And this->HasFlag() is always returning 0 because no style was set to the button when it was created [1].

Basically my patch is just changing this behavior: it set the wxButton style to the style selected by the user in wxGenericFileDirButton constructor. I then need to add a small wxBU_NOTEXT hack to avoid passing a style already used by wxButton.

[1] File picker click : source:wxWidgets/trunk/src/generic/filepickerg.cpp
[2] source:wxWidgets/trunk/include/wx/generic/filepickerg.h

comment:4 Changed 9 years ago by vadz

  • Status changed from new to confirmed

Thanks a lot for the explanation, I understand the problem better now. I think the following patch should fix it as well without any manual checks for style conflicts:

  • include/wx/generic/filepickerg.h

    a b public: 
    7777
    7878protected:
    7979    wxString m_message, m_wildcard;
     80
     81    // we just store the style passed to the ctor here instead of passing it to
     82    // wxButton as some of our bits can conflict with wxButton styles and it
     83    // just doesn't make sense to use picker styles for wxButton anyhow
     84    long m_pickerStyle;
    8085};
    8186
    8287
    public: // overrideable 
    112117    {
    113118        long filedlgstyle = 0;
    114119
    115         if (this->HasFlag(wxFLP_OPEN))
     120        if ( m_pickerStyle & wxFLP_OPEN )
    116121            filedlgstyle |= wxFD_OPEN;
    117         if (this->HasFlag(wxFLP_SAVE))
     122        if ( m_pickerStyle & wxFLP_SAVE )
    118123            filedlgstyle |= wxFD_SAVE;
    119         if (this->HasFlag(wxFLP_OVERWRITE_PROMPT))
     124        if ( m_pickerStyle & wxFLP_OVERWRITE_PROMPT )
    120125            filedlgstyle |= wxFD_OVERWRITE_PROMPT;
    121         if (this->HasFlag(wxFLP_FILE_MUST_EXIST))
     126        if ( m_pickerStyle & wxFLP_FILE_MUST_EXIST )
    122127            filedlgstyle |= wxFD_FILE_MUST_EXIST;
    123         if (this->HasFlag(wxFLP_CHANGE_DIR))
     128        if ( m_pickerStyle & wxFLP_CHANGE_DIR )
    124129            filedlgstyle |= wxFD_CHANGE_DIR;
    125130
    126131        return filedlgstyle;
    public: // overrideable 
    182187    {
    183188        long dirdlgstyle = wxDD_DEFAULT_STYLE;
    184189
    185         if (this->HasFlag(wxDIRP_DIR_MUST_EXIST))
     190        if ( m_pickerStyle & wxDIRP_DIR_MUST_EXIST )
    186191            dirdlgstyle |= wxDD_DIR_MUST_EXIST;
    187         if (this->HasFlag(wxDIRP_CHANGE_DIR))
     192        if ( m_pickerStyle & wxDIRP_CHANGE_DIR )
    188193            dirdlgstyle |= wxDD_CHANGE_DIR;
    189194
    190195        return dirdlgstyle;
  • src/generic/filepickerg.cpp

    diff --git a/src/generic/filepickerg.cpp b/src/generic/filepickerg.cpp
    index f8d7a82..1aaf01a 100644
    a b bool wxGenericFileDirButton::Create(wxWindow *parent, 
    4848                                    const wxString& wildcard,
    4949                                    const wxPoint& pos,
    5050                                    const wxSize& size,
    51                                     long WXUNUSED(style),
     51                                    long style,
    5252                                    const wxValidator& validator,
    5353                                    const wxString& name)
    5454{
     55    m_pickerStyle = style;
     56
    5557    // create this button
    5658    if ( !wxButton::Create(parent, id, label, pos, size, 0, validator, name) )
    5759    {

Could you please check if this works under OS X? TIA!

comment:5 Changed 9 years ago by swiss_kinkajou

I'have tested your code and it works as expected under Mac OSX (10.6.2, x86_64, cocoa). I also agree that the way you propose is clearer compared to mine.

comment:6 Changed 9 years ago by VZ

  • Resolution set to fixed
  • Status changed from confirmed to closed

(In [63654]) Use correct style for the dialog shown by generic file/dir pickers.

The picker styles don't make sense for the button so we don't use them when
creating the button but we do need to somehow use the style the picker was
created with to create an appropriate dialog when it's clicked.

Fix the problem by simply storing the style in a member variable and using it
instead of wxWindow::m_windowStyle.

Closes #11635.

comment:7 Changed 9 years ago by vadz

Thanks for testing!

Note: See TracTickets for help on using tickets.