CFileDialog with OFN_ALLOWMULTISELECT incorrect results for shortcuts

Question

Can someone let me know what am I doing wrong here?

MFC project, I'm using CFileDialog to let user pick multiple files, as such:

CFileDialog fd(TRUE, NULL, NULL,
    OFN_HIDEREADONLY | OFN_OVERWRITEPROMPT | OFN_EXPLORER | OFN_ALLOWMULTISELECT, 
    NULL, this);

if(fd.DoModal() == IDOK)
{
    //Multi-selection
    CString strPaths;
    POSITION fileNamesPosition = fd.GetStartPosition();

    while(fileNamesPosition != NULL)
    {
        if(!strPaths.IsEmpty())
            strPaths += L"\n";

        strPaths += fd.GetNextPathName(fileNamesPosition);
    }  

    AfxMessageBox(strPaths);
}

So if say, there're two shortcut files:

shortcut_1.lnk file that refers to: "D:\Folder\Project_B\Release\Name of Project B.exe"

and shortcut_2.lnk that refers to "D:\Folder\Project_A\Release\Name of Project A.exe"

If I pick both of them from the "File Open" dialog generated by the code above, my resulting strPaths becomes the following, which is incorrect:

D:\Folder\Project_A\Release\Name of Project A.exe
D:\Folder\Project_A\Release\Name of Project B.exe

The second path is wrong!

zett42 · Accepted Answer · 2019-01-22T08:28:28.027

6

Using the GetStartPosition() and GetNextPathName() functions is a mess. For one, they use the old-style API which depends on a correct return buffer size defined via OPENFILENAME struct. MFC does not take care of this! As your question shows, it also has problems with links, even if the buffer size is big enough.

Save yourself a headache and use the Vista+ API, which is available through CFileDialog::GetIFileOpenDialog().

Here is a working code sample:

CFileDialog fd( TRUE, NULL, NULL,
    OFN_HIDEREADONLY | OFN_OVERWRITEPROMPT | OFN_EXPLORER | OFN_ALLOWMULTISELECT,
    NULL, nullptr );

if (fd.DoModal() == IDOK)
{
    //Multi-selection
    CString strPaths;

    CComPtr<IFileOpenDialog> piod = fd.GetIFileOpenDialog();
    ASSERT( piod );

    CComPtr<IShellItemArray> pResults;
    if( SUCCEEDED( piod->GetResults( &pResults ) ) )
    {
        DWORD count = 0; pResults->GetCount( &count );
        for( DWORD i = 0; i < count; ++i )
        {
            CComPtr<IShellItem> pItem;
            if( SUCCEEDED( pResults->GetItemAt( i, &pItem ) ) )
            {
                CComHeapPtr<wchar_t> pPath;
                if( SUCCEEDED( pItem->GetDisplayName( SIGDN_FILESYSPATH, &pPath ) ) )
                {
                    if( !strPaths.IsEmpty() )
                        strPaths += L"\n";
                    strPaths += pPath;
                }
            }
        }
    }

    AfxMessageBox( strPaths );
}

edited Jan 22 '19 at 08:28

answered Jan 21 '19 at 23:32

zett42

25,437
3
35
72

1

Thank you! Some notes re. your code. 1) I'd use my originally provided code as a fallback if your `piod` is null. Otherwise it will crash on WinXP. 2) Small gripe. I'd use `DWORD i` in the `for` loop to match `count` type to prevent warnings. and 3) can I say that every inch of my body **absolutely loathes COM**! It is such an abomination. For instance, what am I suppose to do if any of those five COM functions fail? – c00000fd Jan 22 '19 at 05:39
@c00000fd 1) Make sure to also allocate a big enough return buffer for XP (I've added a link in the 1st paragraph), otherwise users will ge problems when selecting too many files. 2) Thanks, fixed. 3) You do what you do with any error, report it, log it, whatever is appropriate. Personally I would wrap the stuff in a single function that returns `std::vector` and throws a `std::system_error` containing the `HRESULT` and the name of the function that failed. Should be very rare that these function fail, propably out-of-memory situations only. – zett42 Jan 22 '19 at 08:36
Yeah, thanks for the follow up. I was already pre-allocating [32767](http://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx) `wchar_t`'s for the old XP buffer in `fd.m_ofn.lpstrFile` and `fd.m_ofn.nMaxFile` variables. It's just that it didn't return correct results -- that was the issue. But I guess that bug will stay with XP, if someone is still using it. – c00000fd Jan 22 '19 at 21:58
@c00000fd Otherwise Remy has a possible workaround, that should also work under XP. – zett42 Jan 22 '19 at 22:00
Yeah, I thought of using `OFN_NODEREFERENCELINKS` flag myself. It's not very sensible though for the software that I need it for to return a link itself instead of the file that it points to. As for trying to fix it for XP users, their number must be really dwindling now. That OS can't have a large user share these days, can it? When you write your software, what range of OSs do you cover? – c00000fd Jan 22 '19 at 22:04
1

@c00000fd Fortunately my employer doesn't support XP anymore. For private projects I would start with Windows 7 as oldest support OS. – zett42 Jan 22 '19 at 22:07

score 2 · Answer 2 · answered Jan 21 '19 at 23:22

Sounds like a bug in CFileDialog.

Typically, the returned paths are a concatenation of the currently displayed directory path and the selected filenames. In the case of a lnk file, perhaps CFileDialog is extracting just the target filename and concatenating it to the path of the parent folder of the lnk file, rather than just returning the full target path that is inside the lnk file. Hard to say for sure without seeing the actual source code for CFileDialog.

To avoid this behavior, you can include the OFN_NODEREFERENCELINKS flag when invoking the dialog, so you get the full paths to the actual lnk files, and then you can manually resolve their targets using IShellLink after the dialog has been dismissed.

CFileDialog with OFN_ALLOWMULTISELECT incorrect results for shortcuts

2 Answers2