Currently shell32 only transfers the plain icon for `Shell_NotifyIcon` calls, ignoring balloon icons. This patch allows transferring both images to explorer.exe tray.
-- v11: shell32: add support for balloon icon copying shell32: move icon related fields in notify_data into separate struct
From: Sergei Chernyadyev serg.cherniadjev@gmail.com
--- dlls/shell32/systray.c | 37 ++++++++++++++++++++++--------------- programs/explorer/systray.c | 29 ++++++++++++++++++----------- 2 files changed, 40 insertions(+), 26 deletions(-)
diff --git a/dlls/shell32/systray.c b/dlls/shell32/systray.c index 78717cfcdca..0a5d876f14a 100644 --- a/dlls/shell32/systray.c +++ b/dlls/shell32/systray.c @@ -34,12 +34,22 @@
WINE_DEFAULT_DEBUG_CHANNEL(systray);
+struct notify_data_icon +{ + /* data for the icon bitmap */ + UINT width; + UINT height; + UINT planes; + UINT bpp; +}; + struct notify_data /* platform-independent format for NOTIFYICONDATA */ { LONG hWnd; UINT uID; UINT uFlags; UINT uCallbackMessage; + struct notify_data_icon icon_info; /* systray icon bitmap info */ WCHAR szTip[128]; DWORD dwState; DWORD dwStateMask; @@ -51,13 +61,10 @@ struct notify_data /* platform-independent format for NOTIFYICONDATA */ WCHAR szInfoTitle[64]; DWORD dwInfoFlags; GUID guidItem; - /* data for the icon bitmap */ - UINT width; - UINT height; - UINT planes; - UINT bpp; + BYTE icon_data[]; };
+ /************************************************************************* * Shell_NotifyIcon [SHELL32.296] * Shell_NotifyIconA [SHELL32.297] @@ -164,7 +171,7 @@ BOOL WINAPI Shell_NotifyIconW(DWORD dwMessage, PNOTIFYICONDATAW nid) BITMAP bmColour; LONG cbMaskBits; LONG cbColourBits = 0; - char *buffer; + BYTE *buffer;
if (!GetIconInfo(nid->hIcon, &iconinfo)) goto noicon; @@ -192,21 +199,21 @@ BOOL WINAPI Shell_NotifyIconW(DWORD dwMessage, PNOTIFYICONDATAW nid)
data = (struct notify_data *)buffer; memset( data, 0, sizeof(*data) ); - buffer += sizeof(*data); + buffer = &data->icon_data[0]; GetBitmapBits(iconinfo.hbmMask, cbMaskBits, buffer); if (!iconinfo.hbmColor) { - data->width = bmMask.bmWidth; - data->height = bmMask.bmHeight / 2; - data->planes = 1; - data->bpp = 1; + data->icon_info.width = bmMask.bmWidth; + data->icon_info.height = bmMask.bmHeight / 2; + data->icon_info.planes = 1; + data->icon_info.bpp = 1; } else { - data->width = bmColour.bmWidth; - data->height = bmColour.bmHeight; - data->planes = bmColour.bmPlanes; - data->bpp = bmColour.bmBitsPixel; + data->icon_info.width = bmColour.bmWidth; + data->icon_info.height = bmColour.bmHeight; + data->icon_info.planes = bmColour.bmPlanes; + data->icon_info.bpp = bmColour.bmBitsPixel; buffer += cbMaskBits; GetBitmapBits(iconinfo.hbmColor, cbColourBits, buffer); DeleteObject(iconinfo.hbmColor); diff --git a/programs/explorer/systray.c b/programs/explorer/systray.c index b47c2239abd..c3348619ca4 100644 --- a/programs/explorer/systray.c +++ b/programs/explorer/systray.c @@ -36,12 +36,22 @@ WINE_DEFAULT_DEBUG_CHANNEL(systray); #define TRAY_MINIMIZE_ALL 419 #define TRAY_MINIMIZE_ALL_UNDO 416
+struct notify_data_icon +{ + /* data for the icon bitmap */ + UINT width; + UINT height; + UINT planes; + UINT bpp; +}; + struct notify_data /* platform-independent format for NOTIFYICONDATA */ { LONG hWnd; UINT uID; UINT uFlags; UINT uCallbackMessage; + struct notify_data_icon icon_info; /* systray icon bitmap info */ WCHAR szTip[128]; DWORD dwState; DWORD dwStateMask; @@ -53,11 +63,7 @@ struct notify_data /* platform-independent format for NOTIFYICONDATA */ WCHAR szInfoTitle[64]; DWORD dwInfoFlags; GUID guidItem; - /* data for the icon bitmap */ - UINT width; - UINT height; - UINT planes; - UINT bpp; + BYTE icon_data[]; };
#define ICON_DISPLAY_HIDDEN -1 @@ -836,11 +842,13 @@ static BOOL handle_incoming(HWND hwndSource, COPYDATASTRUCT *cds) { struct icon *icon = NULL; const struct notify_data *data; + const BYTE *icon_data; NOTIFYICONDATAW nid; int ret = FALSE;
if (cds->cbData < sizeof(*data)) return FALSE; data = cds->lpData; + icon_data = &data->icon_data[0];
nid.cbSize = sizeof(nid); nid.hWnd = LongToHandle( data->hWnd ); @@ -860,22 +868,21 @@ static BOOL handle_incoming(HWND hwndSource, COPYDATASTRUCT *cds)
/* FIXME: if statement only needed because we don't support interprocess * icon handles */ - if ((nid.uFlags & NIF_ICON) && cds->cbData > sizeof(*data)) + if ((nid.uFlags & NIF_ICON) && cds->cbData > ((char*)icon_data - (char*)data)) { LONG cbMaskBits; LONG cbColourBits; - const char *buffer = (const char *)(data + 1);
- cbMaskBits = (data->width * data->height + 15) / 16 * 2; - cbColourBits = (data->planes * data->width * data->height * data->bpp + 15) / 16 * 2; + cbMaskBits = (data->icon_info.width * data->icon_info.height + 15) / 16 * 2; + cbColourBits = (data->icon_info.planes * data->icon_info.width * data->icon_info.height * data->icon_info.bpp + 15) / 16 * 2;
if (cds->cbData < sizeof(*data) + cbMaskBits + cbColourBits) { ERR( "buffer underflow\n" ); return FALSE; } - nid.hIcon = CreateIcon(NULL, data->width, data->height, data->planes, data->bpp, - buffer, buffer + cbMaskBits); + nid.hIcon = CreateIcon(NULL, data->icon_info.width, data->icon_info.height, data->icon_info.planes, data->icon_info.bpp, + &icon_data[0], &icon_data[cbMaskBits]); }
/* try forwarding to the display driver first */
From: Sergei Chernyadyev serg.cherniadjev@gmail.com
--- dlls/shell32/systray.c | 119 +++++++++++++++++++++++++----------- programs/explorer/systray.c | 32 +++++++++- 2 files changed, 114 insertions(+), 37 deletions(-)
diff --git a/dlls/shell32/systray.c b/dlls/shell32/systray.c index 0a5d876f14a..d012a4f5262 100644 --- a/dlls/shell32/systray.c +++ b/dlls/shell32/systray.c @@ -61,6 +61,7 @@ struct notify_data /* platform-independent format for NOTIFYICONDATA */ WCHAR szInfoTitle[64]; DWORD dwInfoFlags; GUID guidItem; + struct notify_data_icon balloon_icon_info; /* balloon icon bitmap info */ BYTE icon_data[]; };
@@ -132,6 +133,11 @@ BOOL WINAPI Shell_NotifyIconW(DWORD dwMessage, PNOTIFYICONDATAW nid) COPYDATASTRUCT cds; struct notify_data data_buffer; struct notify_data *data = &data_buffer; + ICONINFO iconinfo[2] = { { 0 }, { 0 } }; + BITMAP bmMask[2]; + BITMAP bmColour[2]; + LONG cbMaskBits[2] = { 0, 0}; + LONG cbColourBits[2] = { 0, 0 }; BOOL ret;
TRACE("dwMessage = %ld, nid->cbSize=%ld\n", dwMessage, nid->cbSize); @@ -165,63 +171,104 @@ BOOL WINAPI Shell_NotifyIconW(DWORD dwMessage, PNOTIFYICONDATAW nid) /* FIXME: if statement only needed because we don't support interprocess * icon handles */ if (nid->uFlags & NIF_ICON) + GetIconInfo(nid->hIcon, &iconinfo[0]); + + if ((nid->uFlags & NIF_INFO) && (nid->dwInfoFlags & NIIF_USER)) + GetIconInfo(nid->hBalloonIcon, &iconinfo[1]); + + for (unsigned int i = 0; i < 2; i++) { - ICONINFO iconinfo; - BITMAP bmMask; - BITMAP bmColour; - LONG cbMaskBits; - LONG cbColourBits = 0; - BYTE *buffer; - - if (!GetIconInfo(nid->hIcon, &iconinfo)) - goto noicon; - - if (!GetObjectW(iconinfo.hbmMask, sizeof(bmMask), &bmMask) || - (iconinfo.hbmColor && !GetObjectW(iconinfo.hbmColor, sizeof(bmColour), &bmColour))) + if ((iconinfo[i].hbmMask && !GetObjectW(iconinfo[i].hbmMask, sizeof(bmMask), &bmMask[i])) || + (iconinfo[i].hbmColor && !GetObjectW(iconinfo[i].hbmColor, sizeof(bmColour), &bmColour[i]))) { - DeleteObject(iconinfo.hbmMask); - if (iconinfo.hbmColor) DeleteObject(iconinfo.hbmColor); - goto noicon; + DeleteObject(iconinfo[i].hbmMask); + if (iconinfo[i].hbmColor) DeleteObject(iconinfo[i].hbmColor); + memset(&iconinfo[i], 0, sizeof(iconinfo[i])); + continue; }
- cbMaskBits = (bmMask.bmPlanes * bmMask.bmWidth * bmMask.bmHeight * bmMask.bmBitsPixel + 15) / 16 * 2; - if (iconinfo.hbmColor) - cbColourBits = (bmColour.bmPlanes * bmColour.bmWidth * bmColour.bmHeight * bmColour.bmBitsPixel + 15) / 16 * 2; - cds.cbData = sizeof(*data) + cbMaskBits + cbColourBits; + if (iconinfo[i].hbmMask) + cbMaskBits[i] = (bmMask[i].bmPlanes * bmMask[i].bmWidth * bmMask[i].bmHeight * bmMask[i].bmBitsPixel + 15) / 16 * 2; + if (iconinfo[i].hbmColor) + cbColourBits[i] = (bmColour[i].bmPlanes * bmColour[i].bmWidth * bmColour[i].bmHeight * bmColour[i].bmBitsPixel + 15) / 16 * 2; + + cds.cbData += cbMaskBits[i] + cbColourBits[i]; + } + + if (cds.cbData > sizeof(*data)) + { + char *buffer; + BYTE* image_data_buffer; buffer = malloc(cds.cbData); if (!buffer) { - DeleteObject(iconinfo.hbmMask); - if (iconinfo.hbmColor) DeleteObject(iconinfo.hbmColor); + for (unsigned int i = 0; i < 2; i++) + { + if (iconinfo[i].hbmMask) + DeleteObject(iconinfo[i].hbmMask); + + if (iconinfo[i].hbmColor) + DeleteObject(iconinfo[i].hbmColor); + } SetLastError(E_OUTOFMEMORY); return FALSE; }
data = (struct notify_data *)buffer; + image_data_buffer = &data->icon_data[0]; memset( data, 0, sizeof(*data) ); - buffer = &data->icon_data[0]; - GetBitmapBits(iconinfo.hbmMask, cbMaskBits, buffer); - if (!iconinfo.hbmColor) + + if (iconinfo[0].hbmColor) + { + data->icon_info.width = bmColour[0].bmWidth; + data->icon_info.height = bmColour[0].bmHeight; + data->icon_info.planes = bmColour[0].bmPlanes; + data->icon_info.bpp = bmColour[0].bmBitsPixel; + } + else if (iconinfo[0].hbmMask) { - data->icon_info.width = bmMask.bmWidth; - data->icon_info.height = bmMask.bmHeight / 2; + data->icon_info.width = bmMask[0].bmWidth; + data->icon_info.height = bmMask[0].bmHeight / 2; data->icon_info.planes = 1; data->icon_info.bpp = 1; } - else + + for (unsigned int i = 0; i < 2; i++) { - data->icon_info.width = bmColour.bmWidth; - data->icon_info.height = bmColour.bmHeight; - data->icon_info.planes = bmColour.bmPlanes; - data->icon_info.bpp = bmColour.bmBitsPixel; - buffer += cbMaskBits; - GetBitmapBits(iconinfo.hbmColor, cbColourBits, buffer); - DeleteObject(iconinfo.hbmColor); + struct notify_data_icon* msg_icon_info; + if (!iconinfo[i].hbmMask) + continue; + + msg_icon_info = (i == 0) + ? &data->icon_info + : &data->balloon_icon_info; + + if (iconinfo[i].hbmColor) + { + msg_icon_info->width = bmColour[i].bmWidth; + msg_icon_info->height = bmColour[i].bmHeight; + msg_icon_info->planes = bmColour[i].bmPlanes; + msg_icon_info->bpp = bmColour[i].bmBitsPixel; + } + else + { + msg_icon_info->width = bmMask[i].bmWidth; + msg_icon_info->height = bmMask[i].bmHeight / 2; + msg_icon_info->planes = 1; + msg_icon_info->bpp = 1; + } + + GetBitmapBits(iconinfo[i].hbmMask, cbMaskBits[i], &image_data_buffer[0]); + if (iconinfo[i].hbmColor) + { + GetBitmapBits(iconinfo[i].hbmColor, cbColourBits[i], &image_data_buffer[cbMaskBits[i]]); + DeleteObject(iconinfo[i].hbmColor); + } + DeleteObject(iconinfo[i].hbmMask); + image_data_buffer = &image_data_buffer[cbMaskBits[i] + cbColourBits[i]]; } - DeleteObject(iconinfo.hbmMask); }
-noicon: data->hWnd = HandleToLong( nid->hWnd ); data->uID = nid->uID; data->uFlags = nid->uFlags; diff --git a/programs/explorer/systray.c b/programs/explorer/systray.c index c3348619ca4..8e3da851d45 100644 --- a/programs/explorer/systray.c +++ b/programs/explorer/systray.c @@ -63,6 +63,7 @@ struct notify_data /* platform-independent format for NOTIFYICONDATA */ WCHAR szInfoTitle[64]; DWORD dwInfoFlags; GUID guidItem; + struct notify_data_icon balloon_icon_info; /* balloon icon bitmap info */ BYTE icon_data[]; };
@@ -729,7 +730,17 @@ static BOOL modify_icon( struct icon *icon, NOTIFYICONDATAW *nid ) lstrcpynW( icon->info_title, nid->szInfoTitle, ARRAY_SIZE( icon->info_title )); icon->info_flags = nid->dwInfoFlags; icon->info_timeout = max(min(nid->uTimeout, BALLOON_SHOW_MAX_TIMEOUT), BALLOON_SHOW_MIN_TIMEOUT); - icon->info_icon = nid->hBalloonIcon; + + if (icon->info_icon) DestroyIcon(icon->info_icon); + if ((icon->info_flags & NIIF_ICONMASK) == NIIF_USER) + { + icon->info_icon = CopyIcon(nid->hBalloonIcon); + } + else + { + icon->info_icon = NULL; + } + update_balloon( icon ); } if (icon->state & NIS_HIDDEN) hide_icon( icon ); @@ -883,8 +894,26 @@ static BOOL handle_incoming(HWND hwndSource, COPYDATASTRUCT *cds) } nid.hIcon = CreateIcon(NULL, data->icon_info.width, data->icon_info.height, data->icon_info.planes, data->icon_info.bpp, &icon_data[0], &icon_data[cbMaskBits]); + icon_data = &icon_data[cbMaskBits + cbColourBits]; }
+ if ((nid.uFlags & NIF_INFO) && (nid.dwInfoFlags & NIIF_USER) && cds->cbData > ((char*)icon_data - (char*)data)) + { + /* Balloon icon */ + LONG cbMaskBits; + LONG cbColourBits; + + cbMaskBits = (data->balloon_icon_info.width * data->balloon_icon_info.height + 15) / 16 * 2; + cbColourBits = (data->balloon_icon_info.planes * data->balloon_icon_info.width * data->balloon_icon_info.height * data->balloon_icon_info.bpp + 15) / 16 * 2; + + if (cds->cbData < ((char*)icon_data - (char*)data) + cbMaskBits + cbColourBits) + { + WINE_ERR("buffer underflow\n"); + return FALSE; + } + nid.hBalloonIcon = CreateIcon(NULL, data->balloon_icon_info.width, data->balloon_icon_info.height, data->balloon_icon_info.planes, data->balloon_icon_info.bpp, + &icon_data[0], &icon_data[cbMaskBits]); + } /* try forwarding to the display driver first */ if (cds->dwData == NIM_ADD || !(icon = get_icon( nid.hWnd, nid.uID ))) { @@ -919,6 +948,7 @@ static BOOL handle_incoming(HWND hwndSource, COPYDATASTRUCT *cds)
done: if (nid.hIcon) DestroyIcon( nid.hIcon ); + if (nid.hBalloonIcon) DestroyIcon( nid.hBalloonIcon ); sync_taskbar_buttons(); return ret; }
Hi,
It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.
The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details:
The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=148456
Your paranoid android.
=== debian11b (64 bit WoW report) ===
d3d10core: d3d10core: Timeout
Rémi Bernon (@rbernon) commented about dlls/shell32/systray.c:
data = (struct notify_data *)buffer; memset( data, 0, sizeof(*data) );
buffer += sizeof(*data);
buffer = &data->icon_data[0];
```suggestion:-0+0 buffer = data->icon_data; ```
Rémi Bernon (@rbernon) commented about programs/explorer/systray.c:
{ struct icon *icon = NULL; const struct notify_data *data;
const BYTE *icon_data; NOTIFYICONDATAW nid; int ret = FALSE;
if (cds->cbData < sizeof(*data)) return FALSE; data = cds->lpData;
icon_data = &data->icon_data[0];
```suggestion:-0+0 icon_data = data->icon_data; ```
Rémi Bernon (@rbernon) commented about programs/explorer/systray.c:
/* FIXME: if statement only needed because we don't support interprocess * icon handles */
- if ((nid.uFlags & NIF_ICON) && cds->cbData > sizeof(*data))
- if ((nid.uFlags & NIF_ICON) && cds->cbData > ((char*)icon_data - (char*)data))
You can keep the sizeof I think?
Rémi Bernon (@rbernon) commented about programs/explorer/systray.c:
const char *buffer = (const char *)(data + 1);cbMaskBits = (data->width * data->height + 15) / 16 * 2;cbColourBits = (data->planes * data->width * data->height * data->bpp + 15) / 16 * 2;
cbMaskBits = (data->icon_info.width * data->icon_info.height + 15) / 16 * 2;cbColourBits = (data->icon_info.planes * data->icon_info.width * data->icon_info.height * data->icon_info.bpp + 15) / 16 * 2; if (cds->cbData < sizeof(*data) + cbMaskBits + cbColourBits) { ERR( "buffer underflow\n" ); return FALSE; }
nid.hIcon = CreateIcon(NULL, data->width, data->height, data->planes, data->bpp,buffer, buffer + cbMaskBits);
nid.hIcon = CreateIcon(NULL, data->icon_info.width, data->icon_info.height, data->icon_info.planes, data->icon_info.bpp,&icon_data[0], &icon_data[cbMaskBits]);
```suggestion:-0+0 icon_data, icon_data + cbMaskBits); ```
Rémi Bernon (@rbernon) commented about programs/explorer/systray.c:
} nid.hIcon = CreateIcon(NULL, data->icon_info.width, data->icon_info.height, data->icon_info.planes, data->icon_info.bpp, &icon_data[0], &icon_data[cbMaskBits]);
icon_data = &icon_data[cbMaskBits + cbColourBits];
```suggestion:-0+0 icon_data += cbMaskBits + cbColourBits; ```
Rémi Bernon (@rbernon) commented about programs/explorer/systray.c:
} nid.hIcon = CreateIcon(NULL, data->icon_info.width, data->icon_info.height, data->icon_info.planes, data->icon_info.bpp, &icon_data[0], &icon_data[cbMaskBits]);
icon_data = &icon_data[cbMaskBits + cbColourBits];}
if ((nid.uFlags & NIF_INFO) && (nid.dwInfoFlags & NIIF_USER) && cds->cbData > ((char*)icon_data - (char*)data))
```suggestion:-0+0 if ((nid.uFlags & NIF_INFO) && (nid.dwInfoFlags & NIIF_USER) && cds->cbData > sizeof(*data)) ```
Rémi Bernon (@rbernon) commented about programs/explorer/systray.c:
- if ((nid.uFlags & NIF_INFO) && (nid.dwInfoFlags & NIIF_USER) && cds->cbData > ((char*)icon_data - (char*)data))
- {
/* Balloon icon */LONG cbMaskBits;LONG cbColourBits;cbMaskBits = (data->balloon_icon_info.width * data->balloon_icon_info.height + 15) / 16 * 2;cbColourBits = (data->balloon_icon_info.planes * data->balloon_icon_info.width * data->balloon_icon_info.height * data->balloon_icon_info.bpp + 15) / 16 * 2;if (cds->cbData < ((char*)icon_data - (char*)data) + cbMaskBits + cbColourBits){WINE_ERR("buffer underflow\n");return FALSE;}nid.hBalloonIcon = CreateIcon(NULL, data->balloon_icon_info.width, data->balloon_icon_info.height, data->balloon_icon_info.planes, data->balloon_icon_info.bpp,&icon_data[0], &icon_data[cbMaskBits]);
```suggestion:-0+0 icon_data, icon_data + cbMaskBits); ```
Rémi Bernon (@rbernon) commented about dlls/shell32/systray.c:
/* FIXME: if statement only needed because we don't support interprocess * icon handles */ if (nid->uFlags & NIF_ICON)
GetIconInfo(nid->hIcon, &iconinfo[0]);- if ((nid->uFlags & NIF_INFO) && (nid->dwInfoFlags & NIIF_USER))
GetIconInfo(nid->hBalloonIcon, &iconinfo[1]);- for (unsigned int i = 0; i < 2; i++)
Would be cleaner to use explicit variables for each icon, instead of arrays and loops. You can maybe split some of it into separate helpers beforehand to reduce the diff when adding the balloon icon.
On Tue Sep 17 07:40:40 2024 +0000, Rémi Bernon wrote:
if ((nid.uFlags & NIF_INFO) && (nid.dwInfoFlags & NIIF_USER) && cds->cbData > sizeof(*data))
it could work out in the first case, but not here, as if icon is already present, this condition will always be true
-
Marvin -
Rémi Bernon (＠rbernon) -
Sergei Chernyadyev
-
Sergei Chernyadyev (＠Cherser-s)