Bug 491627 - [win32] Rollover tooltip on TreeItem brings Shell to front
Summary: [win32] Rollover tooltip on TreeItem brings Shell to front
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 3.8.1   Edit
Hardware: PC Windows 7
: P3 major (vote)
Target Milestone: 4.6.3   Edit
Assignee: Markus Keller CLA
QA Contact:
URL:
Whiteboard:
Keywords: usability
Depends on: 509947
Blocks:
  Show dependency tree
 
Reported: 2016-04-13 16:20 EDT by Markus Keller CLA
Modified: 2017-02-08 16:08 EST (History)
4 users (show)

See Also:


Attachments
Snippet15_Bug491627.java (1.91 KB, text/plain)
2016-11-28 13:51 EST, Markus Keller CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Keller CLA 2016-04-13 16:20:26 EDT
Run Snippet15. Expand a few items and make the window as narrow as possible, so that some TreeItem texts get cut on the right.

Place another application window on top of the SWT shell (e.g. notepad.exe), so that some of the cut TreeItems stay visible. While the other application is active, move the mouse up and down over the cut TreeItems. Start slowly and then get quicker.

After a while, the SWT shell comes to front and overlaps the other application, but the shell is not activated and does not take input focus.

In Eclipse, this frequently happens for me when I move the mouse over the Debug view while Eclipse is not active. Since my workbench window is typically maximized, this is as bad as if it would take focus.

We probably didn't get bug reports about this because it's quite hard to track down. You have to move the mouse with a certain speed to make it happen, and it only happens when moving over Trees that don't listen to the MeasureItem event (which explains why it doesn't happen in the many views that use custom draw).
Comment 1 Markus Keller CLA 2016-08-23 06:32:03 EDT
*** Bug 497929 has been marked as a duplicate of this bug. ***
Comment 2 Markus Keller CLA 2016-09-13 08:59:27 EDT
This happens all the time when I have the Breakpoints and Debug views in front. Setting a target milestone to keep it on the radar for 4.7.
Comment 3 Markus Keller CLA 2016-11-23 09:27:26 EST
The problem is the call to SetWindowPos in org.eclipse.swt.widgets.Composite#wmNotify(NMHDR, long, long) line 1934. When I comment that line, the shell doesn't pop to front again. The whole wmNotify method was added with commit a82cf9526e3bd01c73cdc5c3786a7972e25aa4c4 ("oprimtize WM_NOTIFY to get rid of multiple new NMHDR's"), and there's no more information available than the comments in the code.

Tooltips should only be shown for controls in the active window. SetWindowPos (.., OS.HWND_TOPMOST, ..) should not be called on inactive shells.
Comment 4 Niraj Modi CLA 2016-11-25 05:16:04 EST
(In reply to Markus Keller from comment #3)
> The problem is the call to SetWindowPos in
> org.eclipse.swt.widgets.Composite#wmNotify(NMHDR, long, long) line 1934.
> When I comment that line, the shell doesn't pop to front again. The whole
> wmNotify method was added with commit
> a82cf9526e3bd01c73cdc5c3786a7972e25aa4c4 ("oprimtize WM_NOTIFY to get rid of
> multiple new NMHDR's"), and there's no more information available than the
> comments in the code.
> 
> Tooltips should only be shown for controls in the active window.
> SetWindowPos (.., OS.HWND_TOPMOST, ..) should not be called on inactive
> shells.

Markus, thanks for investigating this.
HWND_TOPMOST option places the window at the top of the Z order.
Hence to avoid this issue, HWND_TOPMOST option should be used only when the composite has 'focus'.

With this condition for hasFocus() in place, I am able to get rid of the problem.
Will share a gerrit patch shortly.
Comment 5 Eclipse Genie CLA 2016-11-25 08:40:02 EST
New Gerrit change created: https://git.eclipse.org/r/85763
Comment 6 Niraj Modi CLA 2016-11-28 09:01:57 EST
(In reply to Eclipse Genie from comment #5)
> New Gerrit change created: https://git.eclipse.org/r/85763

Above patch, perfectly fixes the problematic scenario mentioned in below Bugzilla(https://bugs.eclipse.org/bugs/show_bug.cgi?id=497929#c1) and Snippet15 with notepad.exe combination also works fine most of the time(with very occasional failure, not 100% sure on this.)

Markus,
Can you please try out the scenario that you faced with the Debug view in Eclipse ?
Comment 7 Markus Keller CLA 2016-11-28 13:51:49 EST
Created attachment 265622 [details]
Snippet15_Bug491627.java

In the Debug and Help views, the fix worked reliably in my testing. However, after reading the scenarios at the beginning of Composite#wmNotify(..), I think this fix will break some of cases that code is trying to fix. I think we should use "display.getActiveShell () == getShell ()" instead of "isFocusControl ()". That worked just as well in my testing, and it should not affect any of the reparenting secenarios where tooltips really should be shown on top.

Unfortunately, I could also reproduce some remaining problems with Snippet15. I'm attaching a modified Snippet15_Bug491627, which lets me reproduce the remaining problem very easily on my Windows 7 (with system font Tahoma 8). Just run the snippet, point the mouse at "UP!", and then move it up into the partly overlapped tree, and then back down.

When I delete the "SetWindowPos (hdr.hwndFrom, hwndInsertAfter, ..." line, then a breakpoint in SetWindowPos doesn't get hit again after startup. I.e. the remaining problem is a second problem that is not about SetWindowPos. I could not find another trigger line that causes the remaining problem.
Comment 8 Markus Keller CLA 2016-11-28 14:02:16 EST
I've reopened bug 497929 and moved the (updated) Gerrit over to that bug.

Keeping this bug for the Snippet15_Bug491627 scenario and reducing severity for now.
Comment 9 Markus Keller CLA 2016-12-14 17:25:27 EST
Using the two conditional breakpoints given below, I found that the remaining issue is that we still get TTN_SHOW wmNotify events for the inactive shell. Calling SetWindowPos with HWND_NOTOPMOST will raise that shell above the active shell, which is wrong. The fix is to swallow those wrong notifications.

A remaining visible effect of this bug in Windows is that spurious tool tips can show up and disappear in a split second.  This is a mostly harmless feature that can also be observed in the Windows Explorer's navigation tree.

Fixed with http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=13999a569890c90b79a7b2e6ea56ad6015fb86cb

The backport to 4.3.2 will just be the comment and this line:

    if (display.getActiveShell () != getShell ()) return LRESULT.ONE;



Breakpoints and their condition:

- org.eclipse.swt.widgets.Shell#windowProc(long, int, long, long):

if (msg != 0x7f /*WM_GETICON*/
    && msg != 0x200 /*WM_MOUSEFIRST*/
    && msg != 0x84
    && msg != 0x20) {
  System.out.printf("windowProc hwnd = %10s; msg = %6s %2$#8x%n", hwnd, msg);
}
return false;


- org.eclipse.swt.widgets.Tree.wmNotify(NMHDR, long, long):

System.out.printf("hdr.code = %6s %1$#8x%n", hdr.code);
//return false;
return hdr.code == -521;
Comment 10 Niraj Modi CLA 2016-12-15 08:58:47 EST
Thanks Markus, for addressing this tricky bug.

I verified/tested the fix(both in Eclipse help view and with attachment 265622 [details]) in latest 64bit I-Build: I20161215-0130 on Win7.
Comment 11 Markus Keller CLA 2017-02-01 14:30:28 EST
The fix solved this bug, but it caused a regression, see bug 509947.

The backport will have to include http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=fbd77b91884962ff2e926458dc0e799f6fe36f4f
Comment 12 Markus Keller CLA 2017-02-08 16:08:11 EST
Fixed in R4_6_maintenance, including fix for bug 509947:
http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=46408b87ce5bd4a3a8f198bea810396f4f192699