[PATCH 0/1] MR2348: comdlg32: Add parent dir button to "Item" file dialog.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=29912 Now it looks like this (this patch adds the separator & "up" button):  -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2348
From: Florian Will <florian.will(a)gmail.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=29912 --- dlls/comdlg32/itemdlg.c | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/dlls/comdlg32/itemdlg.c b/dlls/comdlg32/itemdlg.c index 95778c2f358..256d3fe99dd 100644 --- a/dlls/comdlg32/itemdlg.c +++ b/dlls/comdlg32/itemdlg.c @@ -40,6 +40,7 @@ #define IDC_NAV_TOOLBAR 200 #define IDC_NAVBACK 201 #define IDC_NAVFORWARD 202 +#define IDC_NAVPARENTFOLDER 203 #include <initguid.h> /* This seems to be another version of IID_IFileDialogCustomize. If @@ -1895,7 +1896,8 @@ static void init_toolbar(FileDialogImpl *This, HWND hwnd) { HWND htoolbar; TBADDBITMAP tbab; - TBBUTTON button[2]; + int view_icons_index, num_toolbar_items = 2; + TBBUTTON button[4]; htoolbar = CreateWindowExW(0, TOOLBARCLASSNAMEW, NULL, TBSTYLE_FLAT | WS_CHILD | WS_VISIBLE, 0, 0, 0, 0, @@ -1905,6 +1907,9 @@ static void init_toolbar(FileDialogImpl *This, HWND hwnd) tbab.nID = IDB_HIST_LARGE_COLOR; SendMessageW(htoolbar, TB_ADDBITMAP, 0, (LPARAM)&tbab); + tbab.nID = IDB_VIEW_LARGE_COLOR; + view_icons_index = SendMessageW(htoolbar, TB_ADDBITMAP, 0, (LPARAM)&tbab); + button[0].iBitmap = HIST_BACK; button[0].idCommand = IDC_NAVBACK; button[0].fsState = TBSTATE_ENABLED; @@ -1919,7 +1924,26 @@ static void init_toolbar(FileDialogImpl *This, HWND hwnd) button[1].dwData = 0; button[1].iString = 0; - SendMessageW(htoolbar, TB_ADDBUTTONSW, 2, (LPARAM)button); + if (view_icons_index >= 0) + { + button[2].iBitmap = 0; + button[2].idCommand = 0; + button[2].fsState = TBSTATE_ENABLED; + button[2].fsStyle = BTNS_SEP; + button[2].dwData = 0; + button[2].iString = 0; + + button[3].iBitmap = view_icons_index + VIEW_PARENTFOLDER; + button[3].idCommand = IDC_NAVPARENTFOLDER; + button[3].fsState = TBSTATE_ENABLED; + button[3].fsStyle = BTNS_BUTTON; + button[3].dwData = 0; + button[3].iString = 0; + + num_toolbar_items += 2; + } + + SendMessageW(htoolbar, TB_ADDBUTTONSW, num_toolbar_items, (LPARAM)button); SendMessageW(htoolbar, TB_SETBUTTONSIZE, 0, MAKELPARAM(24,24)); SendMessageW(htoolbar, TB_AUTOSIZE, 0, 0); } @@ -2160,6 +2184,13 @@ static LRESULT on_browse_forward(FileDialogImpl *This) return FALSE; } +static LRESULT on_browse_parentfolder(FileDialogImpl *This) +{ + TRACE("%p\n", This); + IExplorerBrowser_BrowseToIDList(This->peb, NULL, SBSP_PARENT); + return FALSE; +} + static LRESULT on_command_filetype(FileDialogImpl *This, WPARAM wparam, LPARAM lparam) { if(HIWORD(wparam) == CBN_SELCHANGE) @@ -2218,6 +2249,7 @@ static LRESULT on_wm_command(FileDialogImpl *This, WPARAM wparam, LPARAM lparam) case psh1: return on_command_opendropdown(This, wparam, lparam); case IDC_NAVBACK: return on_browse_back(This); case IDC_NAVFORWARD: return on_browse_forward(This); + case IDC_NAVPARENTFOLDER: return on_browse_parentfolder(This); case IDC_FILETYPE: return on_command_filetype(This, wparam, lparam); default: TRACE("Unknown command.\n"); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2348
I just noticed there is !672, which does the same thing (among other things). It seems inactive though and apparently has some issues. My patch loads/uses the "parent folder" icon a bit differently, I'm not really sure how/why it works in the other patch. Otherwise, that portion of !672 seems to be very similar to my patch, with slightly different naming. Since the "parent dir" button is probably the most pressing usability issue with that dialog and the other commit might be a bit too large, I'm keeping this merge request open. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2348#note_26404
Esme Povirk (@madewokherd) commented about dlls/comdlg32/itemdlg.c:
button[1].dwData = 0; button[1].iString = 0;
- SendMessageW(htoolbar, TB_ADDBUTTONSW, 2, (LPARAM)button); + if (view_icons_index >= 0)
In what situation does the earlier TB_ADDBITMAP call fail? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2348#note_26414
On Wed Mar 8 17:59:26 2023 +0000, Esme Povirk wrote:
In what situation does the earlier TB_ADDBITMAP call fail? I'm not sure, I implemented this by just reading the online docs that say it might return -1 on error, so I thought I should check that case.
Looking at wine's implementation, it might fail when `ImageList_Create` fails because of an allocation failure e.g. in `ImageListImpl_CreateInstance` or `CreateCompatibleDC`? I'm not quite sure how the control flow works in `TOOLBAR_AddBitmap` when using the standard icon sets. Now that you mention it, the other SendMessageW has no error check. I guess failure is extremely unlikely here, so if it's okay to ignore, I'd be happy to drop that error handling. :-) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2348#note_26428
On Wed Mar 8 18:43:23 2023 +0000, Florian Will wrote:
I'm not sure, I implemented this by just reading the online docs that say it might return -1 on error, so I thought I should check that case. Looking at wine's implementation, it might fail when `ImageList_Create` fails because of an allocation failure e.g. in `ImageListImpl_CreateInstance` or `CreateCompatibleDC`? I'm not quite sure how the control flow works in `TOOLBAR_AddBitmap` when using the standard icon sets. Now that you mention it, the other SendMessageW has no error check. I guess failure is extremely unlikely here, so if it's okay to ignore, I'd be happy to drop that error handling. :-) Given that the only thing we can do is silently ignore it anyway, and failure is not expected, I think it's not worth checking.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2348#note_26438
participants (3)
-
Esme Povirk (＠madewokherd) -
Florian Will
-
Florian Will (＠w-flo)