Re: shell32: Solve the problem about 'OK' button is always gray when call SHBrowseForFolder with flags including both BIF_BROWSEFORCOMPUTER and BIF_RETURNONLYFSDIRS

[Sebastian Lackner]

On 05.04.2017 07:25, 许文龙 wrote:
> shell32: Solve the problem about 'OK' button is always gray when call SHBrowseForFolder with flags including both BIF_BROWSEFORCOMPUTER and BIF_RETURNONLYFSDIRS
>
> 0001-shell32-Fix-the-bug-about-the-OK-button-in-folder-br.patch

Thank you for your contribution. The general idea is fine, but here are some
suggestions in order to improve your patch.
> From 65f06c86553e0c31aeef167637ef65af0f1c8754 Mon Sep 17 00:00:00 2001
> From: Xu Wenlong 
> Date: Wed, 29 Mar 2017 15:32:55 +0800
> Subject: shell32: Fix the bug about the 'OK' button in folder-browsing dialog
>  is always gray
> To: wine-patches 
> Reply-To: wine-devel
> Signed-off-by: Xu Wenlong
>
> dlls/shell32/brsfolder.c       | 10 +++++++---
>  dlls/shell32/tests/brsfolder.c |  5 +++++
>  2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/dlls/shell32/brsfolder.c b/dlls/shell32/brsfolder.c
> index 4040407..1728b1f 100644
> --- a/dlls/shell32/brsfolder.c
> +++ b/dlls/shell32/brsfolder.c
> @@ -476,9 +476,13 @@ static void BrsFolder_CheckValidSelection( browse_info *info, LPTV_ITEMDATA lptv
>      DWORD dwAttributes;
>      HRESULT r;
> - if ((lpBrowseInfo->ulFlags & BIF_BROWSEFORCOMPUTER) &&
> -
>    !PIDLIsType(pidl, PT_COMP))
> -
>    bEnabled = FALSE;
> - if (!(lpBrowseInfo->ulFlags & BIF_RETURNONLYFSDIRS))
> - {
> -
>    if ((lpBrowseInfo->ulFlags & BIF_BROWSEFORCOMPUTER) &&
> -
>        !PIDLIsType(pidl, PT_COMP))
> -
>        bEnabled = FALSE;
> - }

You can also write this condition in a single if() statement.
If possible, please also test other combination of flags. BIF_RETURNFSANCESTORS
for example seems to behave similar to BIF_RETURNONLYFSDIRS.
> - if (lpBrowseInfo->ulFlags & BIF_RETURNFSANCESTORS)
>    {
>        dwAttributes = SFGAO_FILESYSANCESTOR | SFGAO_FILESYSTEM;
>
> diff --git a/dlls/shell32/tests/brsfolder.c b/dlls/shell32/tests/brsfolder.c
> index bf29d11..fecca81 100644
> --- a/dlls/shell32/tests/brsfolder.c
> +++ b/dlls/shell32/tests/brsfolder.c
> @@ -351,6 +351,11 @@ static void test_selection(void)
>      pidl = SHBrowseForFolderA(&bi);
>      CoTaskMemFree(pidl);
> - /* test with flag */
> - bi.ulFlags = BIF_BROWSEFORCOMPUTER | BIF_RETURNONLYFSDIRS;
> - pidl = SHBrowseForFolderA(&bi);
> - CoTaskMemFree(pidl);

The test passes here even without the fix above. In general it is better
to write the test in such a way, that it really confirms the change is
correct.
> -
>
> IShellFolder_Release(desktop_object);
>
> CoUninitialize();
>
> -- 2.8.1

Best regards,
Sebastian