Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix/124 arguments tooltip regression second attempt #131

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Source/ExcelDna.IntelliSense/IntelliSenseServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ namespace ExcelDna.IntelliSense
// REMEMBER: COM events are not necessarily safe macro contexts.
public static class IntelliSenseServer
{
const string ServerVersion = "1.7.0"; // TODO: Define and manage this somewhere else
const string ServerVersion = "1.7.1"; // TODO: Define and manage this somewhere else

// NOTE: Do not change these constants in custom versions.
// They are part of the co-operative safety mechanism allowing different add-ins providing IntelliSense to work together safely.
Expand Down
1 change: 1 addition & 0 deletions Source/ExcelDna.IntelliSense/UIMonitor/WinEvents.cs
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ bool IsSupportedWinEvent(WinEvent winEvent)
winEvent == WinEvent.EVENT_SYSTEM_MOVESIZESTART || // Only for the on-demand hook
winEvent == WinEvent.EVENT_SYSTEM_MOVESIZEEND || // Only for the on-demand hook
winEvent == WinEvent.EVENT_OBJECT_SELECTION || // Only for the PopupList
winEvent == WinEvent.EVENT_OBJECT_LOCATIONCHANGE ||
winEvent == WinEvent.EVENT_SYSTEM_CAPTURESTART;
}

Expand Down
58 changes: 50 additions & 8 deletions Source/ExcelDna.IntelliSense/UIMonitor/WindowLocationWatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,39 +9,81 @@ public class WindowLocationWatcher : IDisposable
IntPtr _hWnd;
SynchronizationContext _syncContextAuto;
SynchronizationContext _syncContextMain;
WinEventHook _windowLocationChangeHook;
WinEventHook _windowMoveSizeHook;
WinEventHook _locationChangeEventHook;

public event EventHandler LocationChanged;

// NOTE: An earlier attempt was to monitor LOCATIONCHANGE only between EVENT_SYSTEM_MOVESIZESTART and EVENT_SYSTEM_MOVESIZEEND
// (for the purpose of moving the tooltip to the correct position when the user moves the Excel main window)
// This nearly worked, and meant we were watching many fewer events ...
// ...but we missed some of the resizing events for the window, leaving our tooltip stranded.
// So until we can find a workaround for that (perhaps a timer would work fine for this), we watch all the LOCATIONCHANGE events.
// We then started to watch all the LOCATIONCHANGE events, but it caused the Excel main window to lag when dragging.
// (This drag issue seems to have been introduced with an Office update around November 2022)
// So until we can find a workaround for that (perhaps a timer would work fine for this), we decided not to bother
// with tracking the tooltip position (we still update it as soon as the Excel main window moving ends).
// We still need to watch the LOCATIONCHANGE events, otherwise the tooltip is not shown at all in some cases.
// To workaround the Excel main window lagging, we unhook from LOCATIONCHANGE upon encountering EVENT_SYSTEM_MOVESIZESTART
// and then hook again upon encountering EVENT_SYSTEM_MOVESIZEEND (see UnhookFromLocationChangeUponDraggingExcelMainWindow).
public WindowLocationWatcher(IntPtr hWnd, SynchronizationContext syncContextAuto, SynchronizationContext syncContextMain)
{
_hWnd = hWnd;
_syncContextAuto = syncContextAuto;
_syncContextMain = syncContextMain;
_windowLocationChangeHook = new WinEventHook(WinEventHook.WinEvent.EVENT_SYSTEM_MOVESIZESTART, WinEventHook.WinEvent.EVENT_SYSTEM_MOVESIZEEND, _syncContextAuto, syncContextMain, _hWnd);
_windowLocationChangeHook.WinEventReceived += _windowLocationChangeHook_WinEventReceived;
_windowMoveSizeHook = new WinEventHook(WinEventHook.WinEvent.EVENT_SYSTEM_MOVESIZESTART, WinEventHook.WinEvent.EVENT_SYSTEM_MOVESIZEEND, _syncContextAuto, syncContextMain, _hWnd);
_windowMoveSizeHook.WinEventReceived += _windowMoveSizeHook_WinEventReceived;

SetUpLocationChangeEventListener();
}

void _windowLocationChangeHook_WinEventReceived(object sender, WinEventHook.WinEventArgs winEventArgs)
void SetUpLocationChangeEventListener()
{

_locationChangeEventHook = new WinEventHook(WinEventHook.WinEvent.EVENT_OBJECT_LOCATIONCHANGE, WinEventHook.WinEvent.EVENT_OBJECT_LOCATIONCHANGE, _syncContextAuto, _syncContextMain, IntPtr.Zero);
_locationChangeEventHook.WinEventReceived += _windowMoveSizeHook_WinEventReceived;
}

// This allows us to temporarily stop listening to EVENT_OBJECT_LOCATIONCHANGE events when the user is dragging the Excel main window.
// Otherwise we are going to bump into https://github.com/Excel-DNA/IntelliSense/issues/123. The rest of the time we need to stay
// hooked to EVENT_OBJECT_LOCATIONCHANGE for IntelliSense to work correctly (see https://github.com/Excel-DNA/IntelliSense/issues/124).
void UnhookFromLocationChangeUponDraggingExcelMainWindow(WinEventHook.WinEventArgs e)
{
if (e.EventType == WinEventHook.WinEvent.EVENT_SYSTEM_MOVESIZESTART)
{
_syncContextMain.Post(_ => _locationChangeEventHook?.Dispose(), null);
}

if (e.EventType == WinEventHook.WinEvent.EVENT_SYSTEM_MOVESIZEEND)
{
_syncContextMain.Post(_ => SetUpLocationChangeEventListener(), null);
}
}

void _windowMoveSizeHook_WinEventReceived(object sender, WinEventHook.WinEventArgs winEventArgs)
{
#if DEBUG
Logger.WinEvents.Verbose($"{winEventArgs.EventType} - Window {winEventArgs.WindowHandle:X} ({Win32Helper.GetClassName(winEventArgs.WindowHandle)} - Object/Child {winEventArgs.ObjectId} / {winEventArgs.ChildId} - Thread {winEventArgs.EventThreadId} at {winEventArgs.EventTimeMs}");
#endif

UnhookFromLocationChangeUponDraggingExcelMainWindow(winEventArgs);

LocationChanged?.Invoke(this, EventArgs.Empty);
}

// Runs on the Main thread, perhaps during shutdown
public void Dispose()
{
Debug.Assert(Thread.CurrentThread.ManagedThreadId == 1);
if (_windowLocationChangeHook != null)
if (_windowMoveSizeHook != null)
{
_windowMoveSizeHook.Dispose();
_windowMoveSizeHook = null;
}

if (_locationChangeEventHook != null)
{
_windowLocationChangeHook.Dispose();
_windowLocationChangeHook = null;
_locationChangeEventHook.Dispose();
_locationChangeEventHook = null;
}
}
}
Expand Down
13 changes: 11 additions & 2 deletions Source/ExcelDna.IntelliSense/UIMonitor/WindowWatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@ internal WindowChangedEventArgs(IntPtr windowHandle, WinEventHook.WinEvent winEv
case WinEventHook.WinEvent.EVENT_OBJECT_FOCUS:
Type = ChangeType.Focus;
break;
case WinEventHook.WinEvent.EVENT_OBJECT_LOCATIONCHANGE:
Type = ChangeType.LocationChange;
ObjectId = ChangeObjectId.Caret;
break;
case WinEventHook.WinEvent.EVENT_SYSTEM_MOVESIZEEND:
Type = ChangeType.LocationChange;
break;
Expand Down Expand Up @@ -113,6 +117,9 @@ internal WindowChangedEventArgs(IntPtr windowHandle, WinEventHook.WinEvent winEv
const string _nuiDialogClass = "NUIDialog";
const string _selectDataSourceTitle = "Select Data Source"; // TODO: How does localization work?

readonly SynchronizationContext _syncContextAuto;
readonly SynchronizationContext _syncContextMain;

List<WinEventHook> _windowStateChangeHooks = new List<WinEventHook>();

// These track keyboard focus for Windows in the Excel process
Expand All @@ -134,6 +141,9 @@ internal WindowChangedEventArgs(IntPtr windowHandle, WinEventHook.WinEvent winEv

public WindowWatcher(SynchronizationContext syncContextAuto, SynchronizationContext syncContextMain)
{
_syncContextAuto = syncContextAuto;
_syncContextMain = syncContextMain;

#pragma warning disable CS0618 // Type or member is obsolete (GetCurrentThreadId) - But for debugging we want to monitor this anyway
// Debug.Print($"### WindowWatcher created on thread: Managed {Thread.CurrentThread.ManagedThreadId}, Native {AppDomain.GetCurrentThreadId()}");
#pragma warning restore CS0618 // Type or member is obsolete
Expand All @@ -151,8 +161,6 @@ public WindowWatcher(SynchronizationContext syncContextAuto, SynchronizationCont
// EVENT_OBJECT_SELECTIONREMOVE
// EVENT_OBJECT_SELECTIONWITHIN
// EVENT_OBJECT_STATECHANGE (0x800A = 32778)
// NB: Including the next event 'EVENT_OBJECT_LOCATIONCHANGE (0x800B = 32779)' will cause the Excel main window to lag when dragging.
// This drag issue seems to have been introduced with an Office update around November 2022.
_windowStateChangeHooks.Add(new WinEventHook(WinEventHook.WinEvent.EVENT_OBJECT_CREATE, WinEventHook.WinEvent.EVENT_OBJECT_STATECHANGE, syncContextAuto, syncContextMain, IntPtr.Zero));
_windowStateChangeHooks.Add(new WinEventHook(WinEventHook.WinEvent.EVENT_SYSTEM_CAPTURESTART, WinEventHook.WinEvent.EVENT_SYSTEM_CAPTURESTART, syncContextAuto, syncContextMain, IntPtr.Zero));

Expand Down Expand Up @@ -220,6 +228,7 @@ void _windowStateChangeHook_WinEventReceived(object sender, WinEventHook.WinEven
{
Debug.Fail("WinEvent with window 0!?");
}

if (e.EventType == WinEventHook.WinEvent.EVENT_OBJECT_FOCUS)
{
// Might raise change event for Unfocus
Expand Down