Tree.java: correctly escape & in default Tooltip

[Wittmaxi]

Fix for
eclipse-platform/eclipse.platform.ui#238
eclipse-platform/eclipse.platform.ui#1075

When TreeColumn's are too small, a ToolTip is displayed on hover to reveal the whole Label of an Item (The WIN32-Docs call them In-Place Tooltips).
These ToolTips pass through the Composite-ToolTipText-generator which escapes all & in a Text - allowing to reuse Text from Buttons as Tooltips. This is a well documented Feature of setToolTipText. Since TreeItem's Label does not have Mnemonics, we do not assume that the & should require escaping by the User.

This Commit Escapes all & in default Tooltips that are handled by the default mechanism.

[Wittmaxi]

I am not sure wether using RegExes is an okay thing to do here.
I do not expect this method to be called a lot, hence why I am okay sacrificing a bit of Performance for better code clarity.

This bug might also exist in GTK- or Cocoa-based implementations. I suspect that they have similar fixes.

[github-actions]

Test Results

   546 files  ±0     546 suites  ±0   28m 45s ⏱️ - 1m 12s
 4 412 tests ±0   4 395 ✅ ±0   17 💤 ±0  0 ❌ ±0 
16 718 runs  ±0  16 591 ✅ ±0  127 💤 ±0  0 ❌ ±0 

Results for commit 340306e. ± Comparison against base commit fc94eaf.

♻️ This comment has been updated with latest results.

[Phillipus]

Is that the right place to be changing the contents of the tooltip string? Something tells me that it's not, and perhaps ampersand escaping should be done further up the stack, or by the client.

[Wittmaxi]

@Phillipus Escaping the & definitely can't be done by the client since this ToolTip is automatically generated from the Text. I also don't think explicitly setting a ToolTip is a good idea since this ToolTip is only shown when the TreeColumn is too small - property that is checked for before drawing the ToolTip

[Phillipus]

@Phillipus Escaping the & definitely can't be done by the client since this ToolTip is automatically generated from the Text. I also don't think explicitly setting a ToolTip is a good idea since this ToolTip is only shown when the TreeColumn is too small - property that is checked for before drawing the ToolTip

Yeah, I can see the problem. It's just that when I look at the String toolTipText (NMTTDISPINFO hdr) code it seems to me to be the wrong place to actually change the string. But maybe there is no other place to change it.

Is this also happening on Mac and Linux? Edit - it's OK on Mac.

[github-actions]

Test Results (win32)

   30 files  ±0     30 suites  ±0   4m 28s ⏱️ -49s
4 704 tests ±0  4 629 ✅ ±0  75 💤 ±0  0 ❌ ±0 
1 234 runs  ±0  1 210 ✅ ±0  24 💤 ±0  0 ❌ ±0 

Results for commit 9fa60ff. ± Comparison against base commit 1ba37d4.

♻️ This comment has been updated with latest results.

[HeikoKlare]

I came across this again and again and always thought about alternative solutions but found that it's probably the best and most central place where we can escape the string. @Phillipus do you still have any concerns or should we merge this?

[Phillipus]

@Phillipus do you still have any concerns or should we merge this?

It only works for the tooltip in the first column of a Tree that uses columns. So that needs to be addressed.

[HeikoKlare]

fwiw, I used the snippet below for testing.
When testing I also found another issue: when reordering the table columns, the tooltips of the non-reordered cells are shown. I will have created a follow-up PR for that:

• [Win32] Fix tree tooltips for reordered columns #3366

public static void main (String [] args) {
	Display display = new Display ();
	Shell shell = new Shell(display);
	shell.setLayout(new FillLayout());
	Tree t = new Tree (shell, 0);
	t.setHeaderVisible(true);
	t.setLinesVisible(true);
	TreeColumn col1 = new TreeColumn(t, 0);
	col1.setWidth(30);
	col1.setMoveable(true);
	TreeColumn col2 = new TreeColumn(t, 0);
	col2.setWidth(30);
	col2.setMoveable(true);
	new TreeItem (t, 0).setText(new String [] {"A & B", "C & D"});
	new TreeItem (t, 1).setText(new String [] {"E & F", "G & H"});

	shell.open ();
	while (!shell.isDisposed ()) {
		if (!display.readAndDispatch ()) display.sleep ();
	}
	display.dispose ();
}