FileChooser filter bug

General JUCE discussion
1

I noticed that FileChooser filter bug in native dialogs is still not resolved. 

Here is fix:


juce_win32_FileChooser.cpp
line 221:

        const size_t filterSpaceNumChars = 2048;
        HeapBlock<WCHAR> filters;
        filters.calloc (filterSpaceNumChars);
        //const size_t bytesWritten = 
            filter.copyToUTF16 (filters.getData(), filterSpaceNumChars * sizeof (WCHAR));
        //filter.copyToUTF16 (filters + (bytesWritten / sizeof (WCHAR)),
        //                    ((filterSpaceNumChars - 1) * sizeof (WCHAR) - bytesWritten));

 


juce_gui_basics\native\juce_linux_FileChooser.cpp
line: 132

    if (filters.isNotEmpty() && filters != "*" && filters != "*.*")
    {
        //StringArray tokens;
        //tokens.addTokens (filters, ";,|", "\"");
        //for (int i = 0; i < tokens.size(); ++i)
        //    args.add ("--file-filter=" + tokens[i]);
        StringArray tokens;
        tokens.addTokens(filters.replaceCharacter(';', ' '), "|", "\"");
        for (int i = 0; i < tokens.size(); i += 2)
            args.add("--file-filter=" + tokens[i] + "|" + tokens[i + 1] + "");
    }


Please add this fix to source. I'm getting tired making these modifications everytime I download new juce source.

 

This way filters can be used using standard syntax like this:

            juce::String filters = "All Files|*|"
                "G-Code Files|*.nc;*.tap;*.cnc;*.iso;*.gcode;*.ncf;*.txt|"
                "DXF Files|*.dxf|"
                "PLT/HPGL Files|*.plt;*.plo;*.hg;*.hgl;*.hpg;*.hpgl;*.hpgl2|"
                "Gerber Files|*.gbr;*.gbl;*.gtl;*.gko;*.grb;*.cmp;*.sol;*.plc;*.pls;*.stc;*.sts;*.smc;*.sms;*.ssc;*.sss|"
                "NC Drill Files|*.ncd;*.nc;*.drl;*.drd|"
                "CSV Files|*.csv|"
                "BIN Files|*.bin|"
                "Curve Files|*.curve";
            juce::FileChooser    fc(TRANS("Save"), dir, filters, true);

 

Non native and OSX dialogs do not support multiple filters so this does not affect them.

 

 

2

Hi Kroko,

as I mentioned in this post, JUCE does not support the "|" syntax in a file chooser. This would require implementing in Linux/OSX/Windows. If you have a patch for this then feel free to send it our way.

Fabian 

3

I'm not asking for "|" support. All I'm asking is that juce correctly calls native API.  Currently Juce native filter implementation is incorrect. 

 

Before you dismiss my request check MSDN. You will see that OPENFILENAME lpstrFilter member is not correctly filled in current implementation.

https://msdn.microsoft.com/en-us/library/windows/desktop/ms646839(v=vs.85).aspx

My sugestion fixes this .

 

On Linux zenty requires separate "--file-filter" argument for each extension.  Unfortunately zanity documentation is vague about this and google is full of wrong answers.  I examined zenity source to verify how it works.

My sugestion fixes this.

 

If native API is used correctly like I sugested then (as a side effect) "|" will also work on dialogs that have this support. There will be nothing broken on other dialogs without this support. 

4

Sorry, but it looks to me like your suggestion expects all its input to use this '|' syntax, so all existing code that uses semi-colons would be broken... That's obviously unacceptable in a library, so I'm not sure how you expect us to do this?

5

Please don't discard this so easily. 

Please take couple of minures to see, what my proposal for windows does. Windows implementation already uses "|" syntax. See few lines below my "patch" where all "|" are replaced with \0.

Problem is, that HeapBlock is copied incorrectly. All I want is that data is copied to HeapBlock correctly and as it should be according to MSDN.

Linux (zanity) code also already uses "|" syntax. But it incorrectly parses tokens and generates "--file-filter=" arguments wrong way.  Zanity documentation is vague about how multiple filters are used but I examined their source code.

My "patches" fix windows and zanity multifilter problems. Yes, "|" syntax is used for this. But "|" syntax is already in existing code and is just not implemented correctly in code arround this. If you do this nothing will be broken.

It would be great if multifilter support is added to non native dialogs and other OS. But until it is not, at least windows and zanity (which are just few lines from perfect should be fixed. Everything is already there.

 

 

 

6

Hi Kroko, I'm sorry if you feel that we are not correctly responding to this request. I have spent extensive time looking at your patches. However, I do not understand which bug you are trying to fix (other than adding "|" support). If you look at the documentation of the FileChooser functions in JUCE you will see that we don't mention "|" anywhere 

I believe that adding "|" is a good idea and if you send us patches to implement this (even on a subset of platforms) I'd be happy to review them and add them to JUCE. However, the patches need to be written in a backward compatible way (for users who don't use the "|" syntax) and the mac version at least needs code to discard the "|" portions.

Unfortunately, your proposed patches don't make sense to me. I don't see how removing the copyToUTF16 lines could ever work. Surely you need to convert to UTF-16 on Windows. To be honest, I'm surprised it even works on your computer. The linux version also doesn't work for users who don't use the "|" syntax. In fact it even crashed when I tried it on my computer when tokes.size() is odd (which it can be if you don't use the "|" syntax). 

P.S. You are right that it is confusing that we have the 3 lines in the windows code referring to "|". To be honest, I don't understand why those lines are there and they should probably be removed.

7

Thank you for looking into this.

I see that i need more explaing to do :-)

Currently two platforms in Juce support multiple filters. Windows/Native and Linux/Native (I'm not sure about OSX/Native but I suspect it is not supported). Juce dialogs do not support this.

Unfortunately it is not possible to use multiple filters even if platform supports it because of way filter is processed in juce_win32FileChooser.cpp and juce_linux_FileChooser.cpp.  

 

Windows use OPENFILENAME structure, lpstrFilter member for filter. For example, lpstrFilter for two filters needs to be formated like this:

filter_1_name
\0
filter_1_wildcard1;filter_1_wildcard2;filter_1_wildcard3
\0
filter_2_name
\0
filter_2_wildcard1
\0\0

Each filter can have multiple wildcards, separated with ';'

If you look into my '|' syntax above, this is it except that \0 is replaced with '|' for obvious reasons.

Unfortunately Juce copies filter data twice (with filter.copyToUTF16). I'm sure this mistake is just a typo because there are absolutely no reasons to do this. Only one filter.copyToUTF16 is needed and that is what my "fix" does.

'|' syntax is also used in many other languages (Delphi, C#,...) and seems like obvious choice.

I believe my reasoning for Windows version is solid.

 

Linux is slightly different. Native dialogs use Zanity. Zanity has support for multiple filters if implemented correctly. Arguments should be like this:

--file-filter=filter_1_name|filter_1_wildcard1 filter_1_wildcard2 filter_1_wildcard3
--file-filter=filter_2_name|filter_2_wildcard1 

Each filter needs its own "--file-filter=" word, followed by filter name and '|'. Then multiple wildcards can be listed separated with space. 

My "fix" adds this support and uses same syntax like Windows version. You are correct - I did not check if tokens.size() is odd and you got crash. This needs to be fixed.

 

Non native dialogs don't even show filter on display. They do not support multiple filters. Therefore '|' syntax does not apply to them.

An like I said - I'm not sure about OSX but I suspect it does not support multiple filters. Unfortunately I do not have OSX and I can not check. 

I added screenshot how multiple filters should be displayed.

 

 

 

 

 

 

 

 

8

Hey all, sorry to revive this stale thread, but I’m wondering: is this actually a bug or expected behavior, and if a bug, has this been fixed?

What I’m seeing on Windows 10 is that native dialog doesn’t support filters at all - you get a single file type entry. Is this correct/expected?

To verify, take the DialogsDemo project, find the load file chooser and change the filter string from “*” to something like “*.png,*.jpg” - etc.

The filters seem to work as expected in non-native dialogs, but at least the Windows 10 native dialog (haven’t tested macOS or Linux) will just display a single entry which is just the entire original filter string “*.png,*.jpg”, instead of two separate drop down entries for each file type “*.png” and “*.jpg” (and of course this original, unparsed string filters nothing correctly).

If the code in this thread for native win32 has a fix for supporting native file filters in the dialog, and I can create a version that doesn’t break existing code (I don’t care about ‘|’ syntax support) would anyone consider a PR of this fix if I made it (just essentially adding the current syntax with comma and semicolon, etc. so that it parses correctly and creates multiple entries in the native win32 dialog)?

9

Please see my FR: Improve Native FileChooser File Filter Handling

1 Like