From 5cf9c735cb58ec30b579c2b843fb061c5add2ead Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 18 Nov 2019 19:14:38 +0100 Subject: [PATCH] Handle taskbar updates not removing notification icons from it Fix bug with not being able to update wxTaskBarIcon under MSW after a DPI scale change or [dis]connection of another monitor using different DPI: this resulted in "TaskbarCreated" message being sent by the system, which we handled by trying to create the taskbar icon again. However in this case, recreating it failed, presumably because it still existed, as modifying the existing icon still worked. Change the handle of "TaskbarCreated" to try both adding and updating the icon, as it seems that we can't be sure whether we still have it or not when we get this message. Refactor the existing code to specify the operation to perform when calling the new DoSetIcon(). This actually makes things slightly simpler for it, as it doesn't need to update m_iconAdded inside it any more. Closes #18588. --- include/wx/msw/taskbar.h | 14 ++++++++++ src/msw/taskbar.cpp | 60 ++++++++++++++++++++++++++++++++-------- 2 files changed, 63 insertions(+), 11 deletions(-) diff --git a/include/wx/msw/taskbar.h b/include/wx/msw/taskbar.h index 7859674cc6..9200e9438b 100644 --- a/include/wx/msw/taskbar.h +++ b/include/wx/msw/taskbar.h @@ -65,6 +65,20 @@ protected: wxIcon m_icon; wxString m_strTooltip; +private: + enum Operation + { + Operation_Add, + Operation_Modify, + Operation_TryBoth + }; + + // Implementation of the public SetIcon() which may also be used when we + // don't know if we should add a new icon or modify the existing one. + bool DoSetIcon(const wxIcon& icon, + const wxString& tooltip, + Operation operation); + wxDECLARE_DYNAMIC_CLASS_NO_COPY(wxTaskBarIcon); }; diff --git a/src/msw/taskbar.cpp b/src/msw/taskbar.cpp index 976f5e9197..3121c6c32d 100644 --- a/src/msw/taskbar.cpp +++ b/src/msw/taskbar.cpp @@ -139,6 +139,24 @@ wxTaskBarIcon::~wxTaskBarIcon() // Operations bool wxTaskBarIcon::SetIcon(const wxIcon& icon, const wxString& tooltip) +{ + if ( !DoSetIcon(icon, tooltip, + m_iconAdded ? Operation_Modify : Operation_Add) ) + { + return false; + } + + // We surely have it now, after setting it successfully (we could also have + // had it before, but it's harmless to set this flag again in this case). + m_iconAdded = true; + + return true; +} + +bool +wxTaskBarIcon::DoSetIcon(const wxIcon& icon, + const wxString& tooltip, + Operation operation) { // NB: we have to create the window lazily because of backward compatibility, // old applications may create a wxTaskBarIcon instance before wxApp @@ -167,18 +185,35 @@ bool wxTaskBarIcon::SetIcon(const wxIcon& icon, const wxString& tooltip) wxStrlcpy(notifyData.szTip, tooltip.t_str(), WXSIZEOF(notifyData.szTip)); } - bool ok = Shell_NotifyIcon(m_iconAdded ? NIM_MODIFY - : NIM_ADD, ¬ifyData) != 0; - - if ( !ok ) + switch ( operation ) { - wxLogLastError(wxT("Shell_NotifyIcon(NIM_MODIFY/ADD)")); + case Operation_Add: + if ( !Shell_NotifyIcon(NIM_ADD, ¬ifyData) ) + { + wxLogLastError("Shell_NotifyIcon(NIM_ADD)"); + return false; + } + break; + + case Operation_Modify: + if ( !Shell_NotifyIcon(NIM_MODIFY, ¬ifyData) ) + { + wxLogLastError("Shell_NotifyIcon(NIM_MODIFY)"); + return false; + } + break; + + case Operation_TryBoth: + if ( !Shell_NotifyIcon(NIM_ADD, ¬ifyData) && + !Shell_NotifyIcon(NIM_MODIFY, ¬ifyData) ) + { + wxLogLastError("Shell_NotifyIcon(NIM_ADD/NIM_MODIFY)"); + return false; + } + break; } - if ( !m_iconAdded && ok ) - m_iconAdded = true; - - return ok; + return true; } #if wxUSE_TASKBARICON_BALLOONS @@ -325,8 +360,11 @@ long wxTaskBarIcon::WindowProc(unsigned int msg, { if ( msg == gs_msgRestartTaskbar ) // does the icon need to be redrawn? { - m_iconAdded = false; - SetIcon(m_icon, m_strTooltip); + // We can get this message after the taskbar has been really recreated, + // in which case we need to add our icon anew, or if it just needs to + // be refreshed, in which case the existing icon just needs to be + // updated, so try doing both in DoSetIcon(). + DoSetIcon(m_icon, m_strTooltip, Operation_TryBoth); return 0; }