Bug 205199 - [Win32] setImage(null) on Label overrides text

Label need to change some stylebits when changing from showing image to
text and vica versa. This failed for some change orders e.g. when
calling setText then setImage then remove image with setImage(null)
nothing was rendered in the end.

Includes a example to show result from different invocation orders of
setText and setImage on a single label.

Change-Id: I0c258c19ff131ab4a35d441ffec700ef5eeb7b49
Signed-off-by: Paul Pazderski <paul-eclipse@ppazderski.de>

2 files changed, 133 insertions, 14 deletions

diff --git a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Label.java b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Label.java
index 57fef708ef..e15b7da731 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Label.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Label.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2015 IBM Corporation and others.
+ * Copyright (c) 2000, 2019 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -10,6 +10,7 @@
  *
  * Contributors:
  *     IBM Corporation - initial API and implementation
+ *     Paul Pazderski - Bug 205199: setImage(null) on Label overrides text
  *******************************************************************************/
 package org.eclipse.swt.widgets;
 
@@ -354,12 +355,7 @@ public void setImage (Image image) {
 	if ((style & SWT.SEPARATOR) != 0) return;
 	if (image != null && image.isDisposed()) error(SWT.ERROR_INVALID_ARGUMENT);
 	this.image = image;
-	int bits = OS.GetWindowLong (handle, OS.GWL_STYLE);
-	if ((bits & OS.SS_OWNERDRAW) != OS.SS_OWNERDRAW) {
-		bits &= ~(OS.SS_LEFTNOWORDWRAP | OS.SS_CENTER | OS.SS_RIGHT);
-		bits |= OS.SS_OWNERDRAW;
-		OS.SetWindowLong (handle, OS.GWL_STYLE, bits);
-	}
+	updateStyleBits(image == null);
 	OS.InvalidateRect (handle, null, true);
 }
 
@@ -398,6 +394,7 @@ public void setText (String string) {
 	checkWidget ();
 	if (string == null) error (SWT.ERROR_NULL_ARGUMENT);
 	if ((style & SWT.SEPARATOR) != 0) return;
+	updateStyleBits(true);
 	/*
 	* Feature in Windows.  For some reason, SetWindowText() for
 	* static controls redraws the control, even when the text has
@@ -406,7 +403,23 @@ public void setText (String string) {
 	*/
 	if (string.equals (text)) return;
 	text = string;
-	if (image == null || !IMAGE_AND_TEXT) {
+	string = Display.withCrLf (string);
+	TCHAR buffer = new TCHAR (getCodePage (), string, true);
+	OS.SetWindowText (handle, buffer);
+	if ((state & HAS_AUTO_DIRECTION) != 0) {
+		updateTextDirection (AUTO_TEXT_DIRECTION);
+	}
+}
+
+/**
+ * Update the control's static style bits to reflect the changed image vs. text
+ * situation.
+ *
+ * @param showText if <code>true</code> set required style bits to render text
+ *                 otherwise to render the image
+ */
+void updateStyleBits(boolean showText) {
+	if (showText) {
 		int oldBits = OS.GetWindowLong (handle, OS.GWL_STYLE), newBits = oldBits;
 		newBits &= ~OS.SS_OWNERDRAW;
 		if ((style & SWT.LEFT) != 0) {
@@ -419,12 +432,13 @@ public void setText (String string) {
 		if ((style & SWT.CENTER) != 0) newBits |= OS.SS_CENTER;
 		if ((style & SWT.RIGHT) != 0) newBits |= OS.SS_RIGHT;
 		if (oldBits != newBits) OS.SetWindowLong (handle, OS.GWL_STYLE, newBits);
-	}
-	string = Display.withCrLf (string);
-	TCHAR buffer = new TCHAR (getCodePage (), string, true);
-	OS.SetWindowText (handle, buffer);
-	if ((state & HAS_AUTO_DIRECTION) != 0) {
-		updateTextDirection (AUTO_TEXT_DIRECTION);
+	} else {
+		int bits = OS.GetWindowLong (handle, OS.GWL_STYLE);
+		if ((bits & OS.SS_OWNERDRAW) != OS.SS_OWNERDRAW) {
+			bits &= ~(OS.SS_LEFTNOWORDWRAP | OS.SS_CENTER | OS.SS_RIGHT);
+			bits |= OS.SS_OWNERDRAW;
+			OS.SetWindowLong (handle, OS.GWL_STYLE, bits);
+		}
 	}
 }
 
diff --git a/tests/org.eclipse.swt.tests/ManualTests/org/eclipse/swt/tests/manual/Bug205199_Label_Image_vs_Text.java b/tests/org.eclipse.swt.tests/ManualTests/org/eclipse/swt/tests/manual/Bug205199_Label_Image_vs_Text.java
new file mode 100644
index 0000000000..a8205aeea2
--- /dev/null
+++ b/tests/org.eclipse.swt.tests/ManualTests/org/eclipse/swt/tests/manual/Bug205199_Label_Image_vs_Text.java
@@ -0,0 +1,105 @@
+/*******************************************************************************
+ * Copyright (c) 2019 Paul Pazderski 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:
+ *     Paul Pazderski - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.swt.tests.manual;
+
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.graphics.Color;
+import org.eclipse.swt.graphics.GC;
+import org.eclipse.swt.graphics.Image;
+import org.eclipse.swt.layout.RowLayout;
+import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Display;
+import org.eclipse.swt.widgets.Label;
+import org.eclipse.swt.widgets.Shell;
+
+/**
+ * Snippet to test {@link Label} with different invocation orders of
+ * {@link Label#setText(String)} and {@link Label#setImage(Image)}.
+ * <p>
+ * For Win32 and Bug 205199 when the label set a text then set an image and
+ * later removed the image using <code>setImage(null)</code> (variant 7 in this
+ * example) instead of showing the text again nothing was rendered.
+ * </p>
+ * <p>
+ * The example shows from left to right:
+ * </p>
+ * <ol>
+ * <li>a label only setting text -> text visible</li>
+ * <li>a label only setting image -> image visible</li>
+ * <li>a label first setting text then image -> image visible</li>
+ * <li>a label first setting image then text -> text visible</li>
+ * <li>a label first setting text then image then first text again -> text
+ * visible</li>
+ * <li>a label first setting image then text then first image again -> image
+ * visible</li>
+ * <li>a label first setting text then image then image to null -> text
+ * visible</li>
+ * </ol>
+ */
+public class Bug205199_Label_Image_vs_Text {
+	public static void main(String[] args) {
+		Display display = new Display();
+		Shell shell = new Shell(display);
+
+		Composite container = new Composite(shell, SWT.NONE);
+		container.setLayout(new RowLayout(SWT.HORIZONTAL));
+
+		Image image = new Image(display, 32, 32);
+		Color color = display.getSystemColor(SWT.COLOR_DARK_GREEN);
+		GC gc = new GC(image);
+		gc.setBackground(color);
+		gc.fillRectangle(image.getBounds());
+		gc.dispose();
+
+		Label labelText = new Label(container, SWT.NONE);
+		Label labelImage = new Label(container, SWT.NONE);
+		Label labelTextAndImage = new Label(container, SWT.NONE);
+		Label labelImageAndText = new Label(container, SWT.NONE);
+		Label labelTextAndImageAndText = new Label(container, SWT.NONE);
+		Label labelImageAndTextAndImage = new Label(container, SWT.NONE);
+		Label labelTextAndImageAndImageNull = new Label(container, SWT.NONE);
+
+		labelText.setText("Text");
+
+		labelImage.setImage(image);
+
+		labelTextAndImage.setText("Text");
+		labelTextAndImage.setImage(image);
+
+		labelImageAndText.setImage(image);
+		labelImageAndText.setText("Text");
+
+		labelTextAndImageAndText.setText("Text");
+		labelTextAndImageAndText.setImage(image);
+		labelTextAndImageAndText.setText("Text");
+
+		labelImageAndTextAndImage.setImage(image);
+		labelImageAndTextAndImage.setText("Text");
+		labelImageAndTextAndImage.setImage(image);
+
+		labelTextAndImageAndImageNull.setText("Text");
+		labelTextAndImageAndImageNull.setImage(image);
+		labelTextAndImageAndImageNull.setImage(null);
+
+		container.pack();
+		shell.pack();
+		shell.open();
+		while (!shell.isDisposed()) {
+			if (!display.readAndDispatch())
+				display.sleep();
+		}
+		image.dispose();
+		display.dispose();
+	}
+}