openvpn-gui icon indicating copy to clipboard operation
openvpn-gui copied to clipboard

Can't see ip address in tray tooltip

Open raddyst opened this issue 1 year ago • 8 comments

When i connect with https://github.com/zhovner/zaborona_help/blob/master/openvpn-client-config/srv0.zaborona-help-UDP-no-encryption_maxroutes.ovpn profile,

image

i can't see ip address in tray tooltip.

image

ps. With other profiles - all ok, win7 x64

raddyst avatar Jan 17 '24 21:01 raddyst

@selvanair do we have a message buffer for the whole message which fills up for over-long profile names?

cron2 avatar Jan 17 '24 21:01 cron2

When i connect with https://github.com/zhovner/zaborona_help/blob/master/openvpn-client-config/srv0.zaborona-help-UDP-no-encryption_maxroutes.ovpn profile,

The total text length allowed in Windows "notifyicon" tooltip is limited to 128 characters so truncation happens when the profile name is too long. As the balloon message has more space (256 characters) and less information, truncation is less likely there.

Double click on tray icon to open the status window and see the full IP address.

@cron2

@selvanair do we have a message buffer for the whole message which fills up for over-long profile names?

As space s limited, we do truncate some strings used to build up the tooltip, but in this case the truncation is due to Windows limit.

When there are multiple profiles connected, we try to list all connected/connecting profiles truncating both lists at ~100 characters. The total message would still get truncated by Windows to 128. When there is only one profile connected (the case above), we add "IP address" and "connected since" info as well.

In the latter case, to avoid truncating the IP, we could truncate the profile name to about 32 characters but that works for v4 addresses only.

selvanair avatar Jan 18 '24 03:01 selvanair

PR #670 proposes a fix to this using a custom tooltip window to show the text. If the text is too long [*], the profile name part is truncated to ensure the IP address and time will be always displayed in full.

Tested on Win10 with status bar at bottom, right, top and left. Untested on multiple monitors.

[*] I set it at 500 characters but could be made longer. The built-in tooltip is restricted to 128 characters which is too small to show the IP, time and anything more than a short profile name.

selvanair avatar Feb 10 '24 04:02 selvanair

Hi,

On Windows 11 there are two issues;

  • When there is no connection, there is no tooltip at all. Currently we display OpenVPN GUI there.
  • When there is active connection, tooltip is displayed on top left. Currently we display it next to the tray. Näyttökuva 2024-02-12 110020

Näyttökuva 2024-02-12 110139

lstipakov avatar Feb 12 '24 09:02 lstipakov

On Windows 11 there are two issues;

I'm not totally surprised as I had to do a lot of trial and error to get this work on Windows 10. Use of a tooltip window for on demand display at a specified location is not well documented, and is a bit hackish. At the same time using the tooltip class for this window feels compelling

  • When there is no connection, there is no tooltip at all. Currently we display OpenVPN GUI there.

On Windows 10 I had this problem when the added text is empty (only title remains). Adding a as the text fixed it. Win11 is probably more picky.

  • When there is active connection, tooltip is displayed on top left. Currently we display it next to the tray.

This is strange --- I'll look for a Win11 machine to test and sort this out.

Works well on Win10 -- sigh.. Screenshot from 2024-02-12 10-43-12-1

selvanair avatar Feb 12 '24 15:02 selvanair

Hi,

Looks good on Windows 11. A few nick-picks:

  • The tooltip is placed next to cursor, and not above the tray area as before. But I am not sure if it worth to fix the placement.
  • The title is in bold, unlike before. Is this intended behavior? It looks okayish when we're in connected state and tooltip contains other information, but in the idle state it looks a bit odd, especially when other apps' tooltips are not bold.

Here is a small patch which makes title part of message to remove boldness and moves tip_msh from global to local scope (why it was in global btw?):

 diff --git a/tray.c b/tray.c
index 9e86f53..eed3054 100644
--- a/tray.c
+++ b/tray.c
@@ -50,7 +50,6 @@ HBITMAP hbmpConnecting;
 NOTIFYICONDATA ni;
 HWND traytip; /* handle of tooltip window for tray icon */
 TOOLINFO ti;  /* global tool info structure for tool tip of tray icon*/
-WCHAR tip_msg[500]; /* global buffer for tray tip message */
 
 extern options_t o;
 
@@ -515,9 +514,8 @@ ShowTrayIcon()
         ti.uId = (UINT_PTR) traytip;
         ti.uFlags = TTF_ABSOLUTE|TTF_TRACK|TTF_IDISHWND;
         ti.hwnd = o.hWnd;
-        ti.lpszText = L" ";
+        ti.lpszText = _T(PACKAGE_NAME);
         SendMessage(traytip, TTM_ADDTOOL, 0, (LPARAM)&ti);
-        SendMessage(traytip, TTM_SETTITLE, TTI_NONE, (LPARAM) _T(PACKAGE_NAME));
         SendMessage(traytip, TTM_SETMAXTIPWIDTH, 0, (LPARAM) cx);
     }
 }
@@ -525,6 +523,7 @@ ShowTrayIcon()
 void
 SetTrayIcon(conn_state_t state)
 {
+    TCHAR tip_msg[500] = L"";
     TCHAR msg_connected[100];
     TCHAR msg_connecting[100];
     BOOL first_conn;
@@ -534,7 +533,7 @@ SetTrayIcon(conn_state_t state)
     _tcsncpy(msg_connected, LoadLocalizedString(IDS_TIP_CONNECTED), _countof(msg_connected));
     _tcsncpy(msg_connecting, LoadLocalizedString(IDS_TIP_CONNECTING), _countof(msg_connecting));
 
-    tip_msg[0] = L'\0';
+    _tcsncpy(tip_msg, _T(PACKAGE_NAME), _countof(_T(PACKAGE_NAME)));
     first_conn = TRUE;
     for (connection_t *c = o.chead; c; c = c->next)
     {

If you think we really need bold then I am fine with it.

lstipakov avatar Feb 13 '24 12:02 lstipakov

Good suggestion. In fact I had the same thought of including the title as a part of the message. Simplifies fallback to ni.szTip as well. Will push a commit.

Positioning a fixed amount above the icon looks tricky as the icon location is not returned in NIN_POPUPOPEN message -- also, even stock icons behave in all manners on Windows 10. I'll leave it like this for now, open to improvement if anyone feels like digging into it.

~tip_msg has to be global (or a string literal) though. Otherwise ti.lpszText will be dangling once out of this function. This is not well-documented and my first version had this bug and occasional misbehaviour -- learned the hard way :) But kind of obvious as we assign it in one function and use it elsewhere.~ Well that's what I thought at the time... Looks like the bug I had was something else and no need for a global. The pointer is referred to only when calling TTM_ADDTOOL and TTM_UPDATETIPTEXT.

Will change the patch.

~This was not needed when we used ni.szTip earlier as that is a 128 character buffer in the ni structure.~

selvanair avatar Feb 13 '24 16:02 selvanair

. Looks like the bug I had was something else and no need for a global. The pointer is referred to only when calling TTM_ADDTOOL and TTM_UPDATETIPTEXT.

Yeah but SendMessage is synchronous, so the buffer should only be valid for the duration of the call.

Anyway, PR is now approved and ready to merge,

lstipakov avatar Feb 14 '24 08:02 lstipakov