Bug 565679 - [Win32] SWT requests WS_EX_CLIENTEDGE and then tries to make it look like WS_BORDER

This patch addresses problems in Spinner:

* Border type is now `WS_BORDER` instead if incorrect `WS_EX_CLIENTEDGE`
  `WS_BORDER` is a flat 1px border.
  `WS_EX_CLIENTEDGE` is a 3D 2px border.
  `Spinner` requested wrong `WS_EX_CLIENTEDGE` and then used workarounds
  to convince it to appear as if it had `WS_BORDER`.

* Border is now only present on parent control and not in child controls
  Previously, it was inverse, which is very weird and caused problems,
  which were fought with even more workarounds.

Side effects:

* Text position has changed in Spinner with border.
  That's fine because it now matches text position in Spinner without
  border.

* UpDown control is now 1px narrower in Spinner without border.
  That's fine because it now matches size of UpDown in Spinner with
  border. Previously width was different.

Change-Id: I2d7f9d903423b5fd57d39bf762f3bd995cbc5618
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>

3 files changed, 150 insertions, 33 deletions

diff --git a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Display.java b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Display.java
index b3fc33cfed..a219968fdd 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Display.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Display.java
@@ -215,8 +215,6 @@ public class Display extends Device {
 	boolean useWsBorderLabel = false;
 	static final String USE_WS_BORDER_LIST_KEY       = "org.eclipse.swt.internal.win32.List.use_WS_BORDER"; //$NON-NLS-1$
 	boolean useWsBorderList = false;
-	static final String USE_WS_BORDER_SPINNER_KEY    = "org.eclipse.swt.internal.win32.Spinner.use_WS_BORDER"; //$NON-NLS-1$
-	boolean useWsBorderSpinner = false;
 	static final String USE_WS_BORDER_TABLE_KEY      = "org.eclipse.swt.internal.win32.Table.use_WS_BORDER"; //$NON-NLS-1$
 	boolean useWsBorderTable = false;
 	static final String USE_WS_BORDER_TEXT_KEY       = "org.eclipse.swt.internal.win32.Text.use_WS_BORDER"; //$NON-NLS-1$
@@ -4412,9 +4410,6 @@ public void setData (String key, Object value) {
 		case USE_WS_BORDER_LIST_KEY:
 			useWsBorderList    = !disableCustomThemeTweaks && _toBoolean(value);
 			return;
-		case USE_WS_BORDER_SPINNER_KEY:
-			useWsBorderSpinner = !disableCustomThemeTweaks && _toBoolean(value);
-			return;
 		case USE_WS_BORDER_TABLE_KEY:
 			useWsBorderTable   = !disableCustomThemeTweaks && _toBoolean(value);
 			return;
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Spinner.java b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Spinner.java
index 58cb408964..e0da921f3d 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Spinner.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Spinner.java
@@ -143,7 +143,7 @@ void createHandle () {
 	super.createHandle ();
 	state &= ~(CANVAS | THEME_BACKGROUND);
 	long hInstance = OS.GetModuleHandle (null);
-	int textExStyle = (style & SWT.BORDER) != 0 ? OS.WS_EX_CLIENTEDGE : 0;
+	int textExStyle = 0;
 	int textStyle = OS.WS_CHILD | OS.WS_VISIBLE | OS.ES_AUTOHSCROLL | OS.WS_CLIPSIBLINGS;
 	if ((style & SWT.READ_ONLY) != 0) textStyle |= OS.ES_READONLY;
 	if ((style & SWT.RIGHT_TO_LEFT) != 0) textExStyle |= OS.WS_EX_LAYOUTRTL;
@@ -161,13 +161,6 @@ void createHandle () {
 	OS.SetWindowLongPtr (hwndText, OS.GWLP_ID, hwndText);
 	int upDownStyle = OS.WS_CHILD | OS.WS_VISIBLE | OS.UDS_AUTOBUDDY;
 	if ((style & SWT.WRAP) != 0) upDownStyle |= OS.UDS_WRAP;
-	if ((style & SWT.BORDER) != 0) {
-		if ((style & SWT.RIGHT_TO_LEFT) != 0) {
-			upDownStyle |= OS.UDS_ALIGNLEFT;
-		} else {
-			upDownStyle |= OS.UDS_ALIGNRIGHT;
-		}
-	}
 	hwndUpDown = OS.CreateWindowEx (
 		0,
 		UpDownClass,
@@ -273,11 +266,6 @@ void addVerifyListener (VerifyListener listener) {
 	addListener (SWT.Verify, typedListener);
 }
 
-@Override
-long borderHandle () {
-	return hwndText;
-}
-
 @Override Point computeSizeInPixels (int wHint, int hHint, boolean changed) {
 	checkWidget ();
 	int width = 0, height = 0;
@@ -314,13 +302,15 @@ long borderHandle () {
 	if (width == 0) width = DEFAULT_WIDTH;
 	if (height == 0) height = DEFAULT_HEIGHT;
 	if (wHint != SWT.DEFAULT) width = wHint;
-	if (hHint != SWT.DEFAULT) height = hHint;
-	Rectangle trim = computeTrimInPixels (0, 0, width, height);
-	if (hHint == SWT.DEFAULT) {
-		int upDownHeight = OS.GetSystemMetrics (OS.SM_CYVSCROLL) + 2 * getBorderWidthInPixels ();
-		upDownHeight += (style & SWT.BORDER) != 0 ? 1 : 3;
-		trim.height = Math.max (trim.height, upDownHeight);
+	if (hHint != SWT.DEFAULT)
+		height = hHint;
+	else {
+		int borderAdjustment = (style & SWT.BORDER) != 0 ? -1 : 3;
+		int upDownHeight = OS.GetSystemMetrics (OS.SM_CYVSCROLL);
+		height = Math.max(height, upDownHeight + borderAdjustment);
 	}
+
+	Rectangle trim = computeTrimInPixels (0, 0, width, height);
 	return new Point (trim.width, trim.height);
 }
 
@@ -330,8 +320,16 @@ long borderHandle () {
 	/* Get the trim of the text control */
 	RECT rect = new RECT ();
 	OS.SetRect (rect, x, y, x + width, y + height);
-	int bits0 = OS.GetWindowLong (hwndText, OS.GWL_STYLE);
-	int bits1 = OS.GetWindowLong (hwndText, OS.GWL_EXSTYLE);
+	int bits0 = OS.GetWindowLong (handle, OS.GWL_STYLE);
+	int bits1 = OS.GetWindowLong (handle, OS.GWL_EXSTYLE);
+	/*
+	 * For a very long time, border was WS_EX_CLIENTEDGE. Now that is was
+	 * changed to WS_BORDER, preserve old size for compatibility reasons.
+	 */
+	if ((bits0 & OS.WS_BORDER) != 0) {
+		bits0 &= ~OS.WS_BORDER;
+		bits1 |= OS.WS_EX_CLIENTEDGE;
+	}
 	OS.AdjustWindowRectEx (rect, bits0, false, bits1);
 	width = rect.right - rect.left;
 	height = rect.bottom - rect.top;
@@ -611,7 +609,7 @@ public int getTextLimit () {
 
 @Override
 boolean isUseWsBorder () {
-	return super.isUseWsBorder () || ((display != null) && display.useWsBorderSpinner);
+	return true;
 }
 
 /**
@@ -1136,11 +1134,6 @@ String verifyText (String string, int start, int end, Event keyEvent) {
 }
 
 @Override
-int widgetExtStyle () {
-	return super.widgetExtStyle () & ~OS.WS_EX_CLIENTEDGE;
-}
-
-@Override
 long windowProc (long hwnd, int msg, long wParam, long lParam) {
 	if (hwnd == hwndText || hwnd == hwndUpDown) {
 		LRESULT result = null;
@@ -1233,7 +1226,7 @@ LRESULT WM_SIZE (long wParam, long lParam) {
 	LRESULT result = super.WM_SIZE (wParam, lParam);
 	if (isDisposed ()) return result;
 	int width = OS.LOWORD (lParam), height = OS.HIWORD (lParam);
-	int upDownWidth = OS.GetSystemMetrics (OS.SM_CXVSCROLL);
+	int upDownWidth = OS.GetSystemMetrics (OS.SM_CXVSCROLL) - 1;
 	int textWidth = width - upDownWidth;
 	int border = OS.GetSystemMetrics (OS.SM_CXEDGE);
 	int flags = OS.SWP_NOZORDER | OS.SWP_DRAWFRAME | OS.SWP_NOACTIVATE;
diff --git a/tests/org.eclipse.swt.tests.win32/ManualTests/org/eclipse/swt/tests/win32/snippets/Bug565679_WS_BORDER.java b/tests/org.eclipse.swt.tests.win32/ManualTests/org/eclipse/swt/tests/win32/snippets/Bug565679_WS_BORDER.java
new file mode 100644
index 0000000000..224f0c91ea
--- /dev/null
+++ b/tests/org.eclipse.swt.tests.win32/ManualTests/org/eclipse/swt/tests/win32/snippets/Bug565679_WS_BORDER.java
@@ -0,0 +1,129 @@
+/*******************************************************************************
+ * Copyright (c) 2020 Syntevo and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *     Syntevo - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.swt.tests.win32.snippets;
+
+import org.eclipse.swt.*;
+import org.eclipse.swt.custom.StyledText;
+import org.eclipse.swt.graphics.*;
+import org.eclipse.swt.layout.*;
+import org.eclipse.swt.widgets.*;
+
+public class Bug565679_WS_BORDER {
+	public static void main(String[] args) {
+		final Display display = new Display();
+		final Shell shell = new Shell(display);
+		GridLayout layout = new GridLayout(1, true);
+		layout.horizontalSpacing = 10;
+		shell.setLayout(layout);
+
+		new Label(shell, 0).setText(
+			"Border of these Composite controls is affected by the patch.\n" +
+			"They should have thin border as before."
+		);
+
+		{
+			final Composite compControls = new Composite(shell, 0);
+			compControls.setLayout(new GridLayout(2, true));
+
+			new Label(compControls, 0).setText("Canvas");
+			new Canvas(compControls, SWT.BORDER);
+
+			new Label(compControls, 0).setText("Composite");
+			new Composite(compControls, SWT.BORDER);
+
+			new Label(compControls, 0).setText("StyledText");
+			new StyledText(compControls, SWT.BORDER).setText("Some text");
+
+			new Label(compControls, 0).setText("Spinner (with border)");
+			new Spinner(compControls, SWT.BORDER);
+
+			new Label(compControls, 0).setText("Spinner (no border)");
+			new Spinner(compControls, 0);
+
+			new Label(compControls, 0).setText("Tree (with header)");
+			Tree tree = new Tree(compControls, SWT.BORDER);
+			tree.setHeaderVisible(true);
+			new TreeColumn(tree, 0).setText("Column");
+			new TreeItem(tree, 0).setText("Item");
+		}
+
+		new Label(shell, 0).setText(
+			"Border of these Composite controls is unchanged."
+		);
+
+		{
+			final Composite compControls = new Composite(shell, 0);
+			compControls.setLayout(new GridLayout(2, true));
+
+			new Label(compControls, 0).setText("Combo");
+			new Combo(compControls, SWT.BORDER);
+
+			new Label(compControls, 0).setText("CoolBar");
+			CoolBar coolbar = new CoolBar(compControls, SWT.BORDER);
+			coolbar.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true));
+
+			new Label(compControls, 0).setText("DateTime");
+			new DateTime(compControls, SWT.BORDER);
+
+			new Label(compControls, 0).setText("Decorations");
+			new Decorations(compControls, SWT.BORDER);
+
+			new Label(compControls, 0).setText("ExpandBar");
+			new ExpandBar(compControls, SWT.BORDER);
+
+			new Label(compControls, 0).setText("Table");
+			new Table(compControls, SWT.BORDER);
+
+			new Label(compControls, 0).setText("Text");
+			new Text(compControls, SWT.BORDER);
+
+			new Label(compControls, 0).setText("ToolBar");
+			ToolBar toolbar = new ToolBar(compControls, SWT.BORDER);
+			toolbar.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true));
+
+			new Label(compControls, 0).setText("Tree (no header)");
+			new Tree(compControls, SWT.BORDER);
+		}
+
+		new Label(shell, 0).setText(
+			"Border of these Composite controls is unchanged.\n" +
+			"But using SWT.BORDER with them is very weird anyway..."
+		);
+
+		{
+			final Composite compControls = new Composite(shell, 0);
+			compControls.setLayout(new GridLayout(2, true));
+
+			new Label(compControls, 0).setText("Group");
+			Group group = new Group(compControls, SWT.BORDER);
+			group.setText("Group");
+
+			new Label(compControls, 0).setText("TabFolder");
+			TabFolder tabFolder = new TabFolder(compControls, SWT.BORDER);
+			new TabItem(tabFolder, 0).setText("Item");
+			new TabItem(tabFolder, 0).setText("Item");
+		}
+
+		shell.setSize(400, 800);
+		shell.open();
+
+		while (!shell.isDisposed()) {
+			if (!display.readAndDispatch()) {
+				display.sleep();
+			}
+		}
+
+		display.dispose();
+	}
+}