Fit tooltip of the tree on a single monitor #62 #2142

amartya4256 · 2025-05-12T23:05:20Z

This PR contributes to handling wmNotify event for tooltip of a Tree regardless of if the Tree object maintains the handle of the tooltip in itemToolTipHandle. If there's a tooltip which is not handled by wmNotifyToolTip, it is completely maintained by windows and hence can span over multiple montiors. Hence, with this PR such tooltips are also positioned inside a single monitor to prevent any infinite loop.

Here's the explanation:

For a tree where the tooltip is not maintained in itemToolTipHandle (Since createToolTips was not called -> no createItem(TreeItem, ...) calls or _addListener() call with SWT.Paint or MeasureItem), wmNotifyToolTip cannot handle its drawing and hence it is completely drawn and positioned by windows, which can be risky -> can lead to inifinite loop if the tooltip spans over multiple monitors with different zooms.
```
     else if (hdr.code == OS.TTN_SHOW) {
 	return positionTooltip(hdr, wParam, lParam, false);
 }
```
This block in wmNotify makes sure for such tooltips that all the positioning of any tooltip within a tree is handled by SWT. Hence, it calls for the same positioning logic as of wmNotifyToolTip
According to the old implementation of tooltips, inside positionTooltip method, findCell is called. Inside findCell, those tooltips for which isCustomToolTip is true (they have been registered with a measureItem hook), the toolTip is believed to contain the image and the text while the other case by default doesn't contain the image. It means on the size calculation of a tooltip of a treeItem which has an image but doesnt have a measureItem, would not have the size enough to contain both image and text. Just to make this behaviour consistent, in both the cases we are considering the image size if it available within the treeItem. And hence, it can be used by those toolTips which aren't maintained by Tree in SWT as well. (Line 2814)

Contributes to
#62 and #128

Replaces and thus closes #2142

github-actions · 2025-05-12T23:18:26Z

Test Results

545 files + 6 545 suites +6 31m 11s ⏱️ + 2m 6s
4 380 tests +37 4 362 ✅ +35 18 💤 +3 0 ❌ - 1
16 650 runs +37 16 509 ✅ +35 141 💤 +3 0 ❌ - 1

Results for commit 1a3c66d. ± Comparison against base commit 5cbb48a.

♻️ This comment has been updated with latest results.

fedejeanne

Only small comments but it looks good. @HeikoKlare once the changes are done then I consider this PR approved ✔️

Thx @amartya4256 !

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Tree.java

HeikoKlare

Thank you for the proposal and the nice explanation of how it works.

When testing, I made some observation:

When a different window has focus while I hover over an element having such a tooltip (like in the breakpoints view), it has a wrong size and shows some artifacts. However, I found that we have the same behavior already in the current master state:
To see the tooltip, you need to move the mouse from somewhere outside of the element directly onto the text. If you first hover over the checkbox and then to the text, the tooltip will not show up. That's a slight derivation from existing behavior. Is there a way to fix that?

Finally, the issues linked in the commit seem to be inappropriate. I think the fitting issue is this, isn't it?

Win: Eclipse IDE freeze with Hover across screens with different UI scaling #557

amartya4256 · 2025-05-13T16:27:06Z

Thank you for the proposal and the nice explanation of how it works.

When testing, I made some observation:

When a different window has focus while I hover over an element having such a tooltip (like in the breakpoints view), it has a wrong size and shows some artifacts. However, I found that we have the same behavior already in the current master state:

To see the tooltip, you need to move the mouse from somewhere outside of the element directly onto the text. If you first hover over the checkbox and then to the text, the tooltip will not show up. That's a slight derivation from existing behavior. Is there a way to fix that?

    [
      
    
        ![tooltips_hover_path](https://private-user-images.githubusercontent.com/755472/443238945-96c33098-64bd-4251-9da6-9334ac33d6bb.gif?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NDcxNTM2NzIsIm5iZiI6MTc0NzE1MzM3MiwicGF0aCI6Ii83NTU0NzIvNDQzMjM4OTQ1LTk2YzMzMDk4LTY0YmQtNDI1MS05ZGE2LTkzMzRhYzMzZDZiYi5naWY_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjUwNTEzJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI1MDUxM1QxNjIyNTJaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT1mZGQ3Y2ZiODYzYWVjM2I5MGNjZWFmMjhhNWM5YTI2YTk2NTFkMDEyNmU5NGIxNDdhMzgwN2QzMTk0YWQ1NzQ1JlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCJ9.CPqFZsKIZJwpkrbnMsgr7p3Tx6oEiVPsc-SneeQmw7k)
      ](https://private-user-images.githubusercontent.com/755472/443238945-96c33098-64bd-4251-9da6-9334ac33d6bb.gif?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NDcxNTM2NzIsIm5iZiI6MTc0NzE1MzM3MiwicGF0aCI6Ii83NTU0NzIvNDQzMjM4OTQ1LTk2YzMzMDk4LTY0YmQtNDI1MS05ZGE2LTkzMzRhYzMzZDZiYi5naWY_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjUwNTEzJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI1MDUxM1QxNjIyNTJaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT1mZGQ3Y2ZiODYzYWVjM2I5MGNjZWFmMjhhNWM5YTI2YTk2NTFkMDEyNmU5NGIxNDdhMzgwN2QzMTk0YWQ1NzQ1JlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCJ9.CPqFZsKIZJwpkrbnMsgr7p3Tx6oEiVPsc-SneeQmw7k)
    
    
      
        
          
        
        
          
          
        
      
      [
        
          
        
      ](https://private-user-images.githubusercontent.com/755472/443238945-96c33098-64bd-4251-9da6-9334ac33d6bb.gif?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NDcxNTM2NzIsIm5iZiI6MTc0NzE1MzM3MiwicGF0aCI6Ii83NTU0NzIvNDQzMjM4OTQ1LTk2YzMzMDk4LTY0YmQtNDI1MS05ZGE2LTkzMzRhYzMzZDZiYi5naWY_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjUwNTEzJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI1MDUxM1QxNjIyNTJaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT1mZGQ3Y2ZiODYzYWVjM2I5MGNjZWFmMjhhNWM5YTI2YTk2NTFkMDEyNmU5NGIxNDdhMzgwN2QzMTk0YWQ1NzQ1JlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCJ9.CPqFZsKIZJwpkrbnMsgr7p3Tx6oEiVPsc-SneeQmw7k)

Finally, the issues linked in the commit seem to be inappropriate. I think the fitting issue is this, isn't it?

Win: Eclipse IDE freeze with Hover across screens with different UI scaling #557

For the second point, it doesn't happen because when entering the toolItem with the mouse, we only get one callback from windows, and if it is at the checkmark (in case of unmanaged tooltip), we keep the tooltip hidden and hence when you move to the right side a bit, there's no other callback recieved until you enter the tooltip again. This can be probably fixed by mouse tracking for tree, just like we did in edge tooltips, but I'd propose to do that in a separate ticket. And then we can already merge this. The follow up would be an improvement.

For the first point, it is an existent behavior so I didnt have a look into it.

HeikoKlare · 2025-05-13T19:45:02Z

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Tree.java

+			result = LRESULT.ONE;
+		}
+	} else {
+		// If managedTooltip is false and the cursor is not over the valid part of the


I do not understand this comment: how is managedTooltip relevant here? I don't see that it's checked for this condition branch at all.

I think there should be a check here so that we don't affect anything for custom tooltips.
This else should only execute when managedTooltip == false. I'll make the change.

Although this has no effect on custom tooltips visually, it's better to have that check.

I tested and I can confirm that the extra check if (!managedTooltip) doesn't change the functionality. Furthermore, the whole else block has been added by this PR, which also adds the call to the method positionTooltip(..., false) meaning that this extra check further ensures that the previous use case (which is now the call to positionTooltip(..., true); in line 8150) is not affected.

All in all, I think this change is OK.

fedejeanne

Thank you @amartya4256 !

This PR solves the critical UI freeze problem at a very low cost of not seeing the popup when first hovering on the checkbox and only later on the name of the method (See point Nr. 2 in #2142 (review)).

I accept that trade and propose to look at the introduced behavior in a follow-up PR.

@HeikoKlare if there are no objections on your side, I'll merge this one today.

Marty found a problem. Awaiting improvement

This commit contributes to handling wmNotify event for tooltip of a Tree regardless of if the Tree object maintains the handle of the tooltip in itemToolTipHandle. If there's a tooltip which is not handled by wmNotifyToolTip, it is completely maintained by windows and hence can span over multiple montiors. Hence, with this PR such tooltips are also positioned inside a single monitor to prevent any infinite loop. Contributes to eclipse-platform#62 and eclipse-platform#128

fedejeanne

I tested the latest version (see changes) and I can confirm that the very last issue that @amartya4256 found is fixed.

FTR the issue was triggered like this:

Primary monitor on the right: 175% zoom
Secondary monitor on the left: 125% zoom
Open the workbench on the monitor on the left
Let the popups try to cross over to the monitor on the right --> UI Freeze

The failing check is unrelated and it resembles the failures that we had a few days ago when we had infrastructure issues (403 Forbidden is the hint). The checks for previous the status of this PR were all green so I'll ignore the check for now. Here's the current failure:

Error:  Plugin org.eclipse.tycho:tycho-versions-plugin:4.0.13-SNAPSHOT or one of its dependencies could not be resolved:
Error:  	The following artifacts could not be resolved: org.eclipse.tycho:tycho-versions-plugin:pom:4.0.13-SNAPSHOT (absent): org.eclipse.tycho:tycho-versions-plugin:pom:4.0.13-SNAPSHOT failed to transfer from https://repo.eclipse.org/content/repositories/tycho-snapshots/ during a previous attempt. This failure was cached in the local repository and resolution is not reattempted until the update interval of tycho-snapshots has elapsed or updates are forced. Original error: Could not transfer artifact org.eclipse.tycho:tycho-versions-plugin:pom:4.0.13-SNAPSHOT from/to tycho-snapshots (https://repo.eclipse.org/content/repositories/tycho-snapshots/): status code: 403, reason phrase: Forbidden (403)

So, now it's ready to go on my account.

@HeikoKlare ?

merks · 2025-05-14T11:45:33Z

FYI, there appear to be server problems:

https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/6135

amartya4256 force-pushed the amartya4256/tree_tooltip_fit_generic branch from 9ce016e to 71e41cf Compare May 13, 2025 09:25

amartya4256 mentioned this pull request May 13, 2025

Always manage custom tooltip in Tree in Win32 #62 #2137

Closed

fedejeanne reviewed May 13, 2025

View reviewed changes

HeikoKlare reviewed May 13, 2025

View reviewed changes

amartya4256 linked an issue May 13, 2025 that may be closed by this pull request

Tooltip in Breakpoints freezing the IDE on hover if the tooltip spans multiple monitors vi-eclipse/Eclipse-Platform#300

Closed

amartya4256 force-pushed the amartya4256/tree_tooltip_fit_generic branch from 71e41cf to 4593ef6 Compare May 13, 2025 16:22

HeikoKlare reviewed May 13, 2025

View reviewed changes

akurtakov added the Windows Happens on Windows OS label May 14, 2025

amartya4256 force-pushed the amartya4256/tree_tooltip_fit_generic branch from 4593ef6 to 08f99b7 Compare May 14, 2025 07:49

fedejeanne previously approved these changes May 14, 2025

View reviewed changes

amartya4256 force-pushed the amartya4256/tree_tooltip_fit_generic branch from 08f99b7 to 1a3c66d Compare May 14, 2025 11:18

fedejeanne approved these changes May 14, 2025

View reviewed changes

fedejeanne merged commit 65a88c6 into eclipse-platform:master May 14, 2025
13 of 17 checks passed

fedejeanne deleted the amartya4256/tree_tooltip_fit_generic branch May 14, 2025 12:09

amartya4256 mentioned this pull request May 14, 2025

Fit Table Tooltip in a single monitor for Win32 #2154

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fit tooltip of the tree on a single monitor #62 #2142

Fit tooltip of the tree on a single monitor #62 #2142

Uh oh!

amartya4256 commented May 12, 2025 •

edited by HeikoKlare

Loading

Uh oh!

github-actions bot commented May 12, 2025 •

edited

Loading

Uh oh!

fedejeanne left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HeikoKlare left a comment

Uh oh!

amartya4256 commented May 13, 2025

Uh oh!

HeikoKlare May 13, 2025

Uh oh!

amartya4256 May 14, 2025

Uh oh!

amartya4256 May 14, 2025

Uh oh!

fedejeanne May 14, 2025 •

edited

Loading

Uh oh!

fedejeanne left a comment

Uh oh!

fedejeanne left a comment

Uh oh!

merks commented May 14, 2025

Uh oh!

Uh oh!

Uh oh!

Fit tooltip of the tree on a single monitor #62 #2142

Fit tooltip of the tree on a single monitor #62 #2142

Uh oh!

Conversation

amartya4256 commented May 12, 2025 • edited by HeikoKlare Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

fedejeanne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HeikoKlare left a comment

Choose a reason for hiding this comment

Uh oh!

amartya4256 commented May 13, 2025

Uh oh!

HeikoKlare May 13, 2025

Choose a reason for hiding this comment

Uh oh!

amartya4256 May 14, 2025

Choose a reason for hiding this comment

Uh oh!

amartya4256 May 14, 2025

Choose a reason for hiding this comment

Uh oh!

fedejeanne May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fedejeanne left a comment

Choose a reason for hiding this comment

Uh oh!

fedejeanne left a comment

Choose a reason for hiding this comment

Uh oh!

merks commented May 14, 2025

Uh oh!

Uh oh!

Uh oh!

amartya4256 commented May 12, 2025 •

edited by HeikoKlare

Loading

github-actions bot commented May 12, 2025 •

edited

Loading

fedejeanne May 14, 2025 •

edited

Loading