[PATCH v4 0/1] MR8367: shell32: Try to lookup icon, with fallback on failure

[James McDonnell (＠ElementalWarrior)]

Try to lookup icon information from shell32.dll. If an icon is not
found, default to the file icon.

Instead of defaulting to -1, which was failing for Affinity Photo's
installer.

--
  v4: shell32: Try to lookup icon, with fallback on failure

https://gitlab.winehq.org/wine/wine/-/merge_requests/8367

[PATCH v4 1/1] shell32: Try to lookup icon, with fallback on failure

[James McDonnell]

From: James McDonnell

Try to lookup icon information from shell32.dll. If an icon is not
found, default to the help icon.

This resolves the issue where in affinity photos installer it
would crash because hIcon was returned as 0
---
 dlls/shell32/iconcache.c | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/dlls/shell32/iconcache.c b/dlls/shell32/iconcache.c
index 8f9519d2ca8..edcaf1a576e 100644
--- a/dlls/shell32/iconcache.c
+++ b/dlls/shell32/iconcache.c
@@ -976,6 +976,16 @@ INT WINAPI SHGetIconOverlayIndexW(LPCWSTR pszIconPath, INT iIconIndex)
   return -1;
 }

+/****************************************************************************
+ * StockIconIdToResourceId    [SHELL32.@]
+ *
+ * Map the SHSTOCKICONID value to shresdef.h IDI_ values
+ */
+INT StockIconIdToResourceId(SHSTOCKICONID id)
+{
+  return id + 1;
+}
+
 /****************************************************************************
  * SHGetStockIconInfo [SHELL32.@]
  *
@@ -1000,19 +1010,25 @@ HRESULT WINAPI SHGetStockIconInfo(SHSTOCKICONID id, UINT flags, SHSTOCKICONINFO

GetSystemDirectoryW(sii->szPath, MAX_PATH);

-    /* no icons defined: use default */
-    sii->iIcon = -IDI_SHELL_FILE;
+    sii->iIcon = id;
     lstrcatW(sii->szPath, L"\shell32.dll");

-    if (flags)
-        FIXME("flags 0x%x not implemented\n", flags);
+    if (flags & ~SHGSI_ICON)
+        FIXME("unhandled flags 0x%x\n", flags);

sii->hIcon = NULL;
-    if (flags & SHGSI_ICON)
-        sii->hIcon = LoadIconW(GetModuleHandleW(sii->szPath), MAKEINTRESOURCEW(sii->iIcon));
+    if (flags & SHGSI_ICON) {
+        sii->hIcon = LoadIconW(GetModuleHandleW(sii->szPath), MAKEINTRESOURCEW(StockIconIdToResourceId(id)));
+
+        // many icons do not exist yet in wine, map to the question mark
+        if (!sii->hIcon) {
+          sii->iIcon = IDI_SHELL_HELP - 1;
+          sii->hIcon = LoadIconW(GetModuleHandleW(sii->szPath), MAKEINTRESOURCEW(IDI_SHELL_HELP));
+        }
+    }
     sii->iSysImageIndex = -1;

-    TRACE("%3d: returning %s (%d)\n", id, debugstr_w(sii->szPath), sii->iIcon);
+    TRACE("%3d: returning %s (%d), icon handle=%p\n", id, debugstr_w(sii->szPath), sii->iIcon, sii->hIcon);

return S_OK;
 }

[James McDonnell (＠ElementalWarrior)]

Actually what you were suggesting is just effectively adding 1 to id.

I am still defaulting to another icon if it cannot load one. Because many icons are not handled in wine yet.

Thanks for reviewing @julliard, let me know your thoughts.