Bug 384851 - [CTabFolder] Top right control destroys border for last

CTabItem in 4.2

If the setTopRight control with style SWT.FILL is the only control apart
from the actual CTabItems - i.e. no min/max button, no visible chevron -
the required spacing for the topRight control is missing.

The consequence is that the topRight control is moved left on top of the
last item or outside the tab folder bar if no item available. With an
item available this is barely visible because the spacing is only 3
pixel.

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

3 files changed, 230 insertions, 13 deletions

diff --git a/bundles/org.eclipse.swt/Eclipse SWT Custom Widgets/common/org/eclipse/swt/custom/CTabFolder.java b/bundles/org.eclipse.swt/Eclipse SWT Custom Widgets/common/org/eclipse/swt/custom/CTabFolder.java
index d2f617ce0f..7ee1a40016 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT Custom Widgets/common/org/eclipse/swt/custom/CTabFolder.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT Custom Widgets/common/org/eclipse/swt/custom/CTabFolder.java
@@ -528,6 +528,7 @@ Rectangle[] computeControlBounds (Point size, boolean[][] position) {
 	int x = borderLeft + SPACING;
 	int rightWidth = 0;
 	int allWidth = 0;
+	boolean spacingRight = false;
 	for (int i = 0; i < controls.length; i++) {
 		Point ctrlSize = tabControlSize[i] = !controls[i].isDisposed() && controls[i].getVisible() ? controls[i].computeSize(SWT.DEFAULT, SWT.DEFAULT) : new Point(0,0);
 		int alignment = controlAlignments[i];
@@ -539,6 +540,9 @@ Rectangle[] computeControlBounds (Point size, boolean[][] position) {
 			x += ctrlSize.x;
 			leftWidth += ctrlSize.x;
 		} else {
+			if ((alignment & SWT.WRAP) == 0 && ctrlSize.x > 0) {
+				spacingRight = true;
+			}
 			if ((alignment & (SWT.FILL | SWT.WRAP)) == 0) {
 				rightWidth += ctrlSize.x;
 			}
@@ -554,7 +558,7 @@ Rectangle[] computeControlBounds (Point size, boolean[][] position) {
 
 	int maxWidth = size.x - borderLeft - leftWidth - borderRight;
 	int availableWidth = Math.max(0, maxWidth - itemWidth - rightWidth);
-	if (rightWidth > 0) availableWidth -= SPACING * 2;
+	if (spacingRight) availableWidth -= SPACING * 2;
 	x =  size.x  - borderRight - SPACING;
 	if (itemWidth + allWidth <= maxWidth) {
 		//Everything fits
diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/SwtTestUtil.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/SwtTestUtil.java
index bc78b2fbc5..de3d72675e 100644
--- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/SwtTestUtil.java
+++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/SwtTestUtil.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2018 IBM Corporation and others.
+ * Copyright (c) 2000, 2020 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -17,9 +17,15 @@ package org.eclipse.swt.tests.junit;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
+import java.util.concurrent.atomic.AtomicBoolean;
+
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.SWTError;
 import org.eclipse.swt.SWTException;
+import org.eclipse.swt.events.PaintEvent;
+import org.eclipse.swt.events.PaintListener;
+import org.eclipse.swt.widgets.Display;
+import org.eclipse.swt.widgets.Shell;
 
 public class SwtTestUtil {
 	/**
@@ -103,4 +109,52 @@ public static void assertSWTProblem(String message, int expectedCode, Throwable
 public static boolean isBidi() {
 	return true;
 }
+
+/**
+ * Open the given shell and wait until it is actually opened! Dependent on
+ * platform the shell is not immediately open after {@link Shell#open()} and has
+ * not its final bounds.
+ * <p>
+ * If opening the shell fails or is, for whatever reason, not recognized the
+ * method will return after a short timeout.
+ * </p>
+ *
+ * @param shell the shell to open. Does nothing if <code>null</code> or already
+ *              open.
+ */
+public static void openShell(Shell shell) {
+	if (shell != null && !shell.getVisible()) {
+		if (isGTK) {
+			AtomicBoolean paintRequested = new AtomicBoolean(false);
+			shell.addPaintListener(new PaintListener() {
+				@Override
+				public void paintControl(PaintEvent e) {
+					paintRequested.set(true);
+					shell.removePaintListener(this);
+				}
+			});
+			shell.open();
+			long start = System.currentTimeMillis();
+			while (!paintRequested.get() && System.currentTimeMillis() - 1000 < start) {
+				processEvents();
+			}
+		} else {
+			shell.open();
+		}
+
+	}
+}
+
+/**
+ * Dispatch all pending events (until {@link Display#readAndDispatch()} returned
+ * <code>false</code>).
+ */
+public static void processEvents() {
+	Display display = Display.getCurrent();
+	if (display != null && !display.isDisposed()) {
+		while (display.readAndDispatch()) {
+		}
+	}
+}
+
 }
diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_custom_CTabFolder.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_custom_CTabFolder.java
index 9ace4824be..eeced27d3c 100644
--- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_custom_CTabFolder.java
+++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_custom_CTabFolder.java
@@ -14,13 +14,18 @@
 package org.eclipse.swt.tests.junit;
 
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
+import java.lang.reflect.Field;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
+import java.util.function.BiConsumer;
 
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.custom.CTabFolder;
@@ -30,15 +35,19 @@ import org.eclipse.swt.graphics.Color;
 import org.eclipse.swt.graphics.Font;
 import org.eclipse.swt.graphics.FontData;
 import org.eclipse.swt.graphics.Image;
+import org.eclipse.swt.graphics.Rectangle;
 import org.eclipse.swt.layout.FillLayout;
 import org.eclipse.swt.layout.GridData;
 import org.eclipse.swt.layout.GridLayout;
+import org.eclipse.swt.widgets.Composite;
 import org.eclipse.swt.widgets.Control;
 import org.eclipse.swt.widgets.Display;
 import org.eclipse.swt.widgets.Event;
+import org.eclipse.swt.widgets.Label;
 import org.eclipse.swt.widgets.Text;
 import org.eclipse.swt.widgets.ToolBar;
 import org.eclipse.swt.widgets.ToolItem;
+import org.eclipse.swt.widgets.Widget;
 import org.junit.Before;
 import org.junit.Ignore;
 import org.junit.Test;
@@ -91,10 +100,24 @@ public void test_setFontLorg_eclipse_swt_graphics_Font() {
 /* custom */
 protected CTabFolder ctabFolder;
 
+/**
+ * Dispose all widgets in shell and create a new empty {@link CTabFolder}.
+ */
 private void makeCleanEnvironment() {
-// this method must be private or protected so the auto-gen tool keeps it
-	if (ctabFolder != null) ctabFolder.dispose();
-	ctabFolder = new CTabFolder(shell, 0);
+	makeCleanEnvironment(SWT.NONE);
+}
+
+/**
+ * Dispose all widgets in shell and create a new empty {@link CTabFolder}.
+ *
+ * @param style style bits for the {@link CTabFolder}
+ */
+private void makeCleanEnvironment(int style) {
+	Control[] children = shell.getChildren();
+	for (Control child : children) {
+		child.dispose();
+	}
+	ctabFolder = new CTabFolder(shell, style);
 	setWidget(ctabFolder);
 }
 
@@ -259,14 +282,7 @@ public void test_chevronAppearanceChanged() {
 	createTabFolder(null);
 	shell.open();
 	processEvents();
-	int itemWidth = 0;
-	for (CTabItem item : ctabFolder.getItems()) {
-		itemWidth += item.getBounds().width;
-	}
-	// resize shell to force a chevron
-	shell.setSize(itemWidth*3/4, shell.getSize().y);
-	ToolItem chevron = getChevron(ctabFolder);
-	assertNotNull("Chevron not shown", chevron);
+	ToolItem chevron = showChevron();
 
 	Image oldChevronImg = new Image(display, chevron.getImage(), SWT.IMAGE_COPY);
 	try {
@@ -289,6 +305,82 @@ public void test_chevronAppearanceChanged() {
 	}
 }
 
+@Test
+public void test_childControlOverlap() {
+	BiConsumer<Control, Integer> setTopRightAndCheckOverlap = (control, style) -> {
+		if (style == 0) {
+			ctabFolder.setTopRight(control);
+		} else {
+			ctabFolder.setTopRight(control, style);
+		}
+		processEvents();
+		checkElementOverlap(ctabFolder);
+	};
+
+	makeCleanEnvironment(SWT.CLOSE);
+	shell.setLayout(new FillLayout());
+	SwtTestUtil.openShell(shell);
+
+	Label topRightControl = new Label(ctabFolder, SWT.BORDER);
+	topRightControl.setText("TopRight");
+
+	// Extra test for bug 384851. TopRight control set but no items.
+	setTopRightAndCheckOverlap.accept(topRightControl, SWT.FILL);
+	setTopRightAndCheckOverlap.accept(null, 0);
+
+	for (int i = 1; i <= 5; i++) {
+		CTabItem tabItem = new CTabItem(ctabFolder, SWT.NONE);
+		tabItem.setText("Item" + i);
+		Composite filler = new Composite(ctabFolder, SWT.NONE);
+		filler.setBackground(filler.getDisplay().getSystemColor(SWT.COLOR_GREEN + i));
+		tabItem.setControl(filler);
+	}
+
+	checkElementOverlap(ctabFolder);
+
+	setTopRightAndCheckOverlap.accept(topRightControl, 0);
+	setTopRightAndCheckOverlap.accept(topRightControl, SWT.RIGHT | SWT.WRAP);
+	setTopRightAndCheckOverlap.accept(topRightControl, SWT.FILL);
+	setTopRightAndCheckOverlap.accept(null, 0);
+
+	ctabFolder.setMinimizeVisible(true);
+	checkElementOverlap(ctabFolder);
+
+	setTopRightAndCheckOverlap.accept(topRightControl, 0);
+	setTopRightAndCheckOverlap.accept(topRightControl, SWT.RIGHT | SWT.WRAP);
+	setTopRightAndCheckOverlap.accept(topRightControl, SWT.FILL);
+	setTopRightAndCheckOverlap.accept(null, 0);
+
+	ctabFolder.setMinimizeVisible(false);
+	ctabFolder.setMaximizeVisible(true);
+	checkElementOverlap(ctabFolder);
+
+	setTopRightAndCheckOverlap.accept(topRightControl, 0);
+	setTopRightAndCheckOverlap.accept(topRightControl, SWT.RIGHT | SWT.WRAP);
+	setTopRightAndCheckOverlap.accept(topRightControl, SWT.FILL);
+	setTopRightAndCheckOverlap.accept(null, 0);
+
+	ctabFolder.setMinimizeVisible(true);
+	checkElementOverlap(ctabFolder);
+
+	setTopRightAndCheckOverlap.accept(topRightControl, 0);
+	setTopRightAndCheckOverlap.accept(topRightControl, SWT.RIGHT | SWT.WRAP);
+	setTopRightAndCheckOverlap.accept(topRightControl, SWT.FILL);
+	setTopRightAndCheckOverlap.accept(null, 0);
+
+	// some extra tests including chevron
+	showChevron();
+	checkElementOverlap(ctabFolder);
+	ctabFolder.setMinimizeVisible(true);
+	checkElementOverlap(ctabFolder);
+	ctabFolder.setMaximizeVisible(true);
+	checkElementOverlap(ctabFolder);
+	setTopRightAndCheckOverlap.accept(topRightControl, 0);
+	setTopRightAndCheckOverlap.accept(topRightControl, SWT.RIGHT | SWT.WRAP);
+	setTopRightAndCheckOverlap.accept(topRightControl, SWT.FILL);
+	setTopRightAndCheckOverlap.accept(null, 0);
+}
+
 private void processEvents() {
 	Display display = shell.getDisplay();
 
@@ -317,6 +409,26 @@ private static boolean reflection_shouldHighlight(CTabFolder partStackTabs) {
 }
 
 /**
+ * Resize shell so that the ctabFolder must show the chevron.
+ * <p>
+ * Throws exception on failure or if chevron not found.
+ * </p>
+ *
+ * @return the chevron widget
+ */
+private ToolItem showChevron() {
+	int itemWidth = 0;
+	for (CTabItem item : ctabFolder.getItems()) {
+		itemWidth += item.getBounds().width;
+	}
+	// resize shell to force a chevron
+	shell.setSize(itemWidth*3/4, shell.getSize().y);
+	ToolItem chevron = getChevron(ctabFolder);
+	assertNotNull("Chevron not shown", chevron);
+	return chevron;
+}
+
+/**
  * Find chevron item in CTabFolder. Does not use reflection but searches for a
  * control which seem to be the chevron based on button style and tooltip text.
  *
@@ -339,4 +451,51 @@ private ToolItem getChevron(CTabFolder tabFolder) {
 	return null;
 }
 
+private void checkElementOverlap(CTabFolder tabFolder) {
+	Rectangle folderBounds = tabFolder.getBounds();
+	ArrayList<Widget> subControls = new ArrayList<>();
+	subControls.addAll(Arrays.asList(reflection_getChildControls(tabFolder)));
+	subControls.addAll(Arrays.asList(tabFolder.getItems()));
+	for (int i = 0; i < subControls.size(); i++) {
+		Rectangle boundsA = null;
+		if (subControls.get(i) instanceof Control) {
+			if (!((Control) subControls.get(i)).isVisible())
+				continue;
+			boundsA = ((Control) subControls.get(i)).getBounds();
+		} else if (subControls.get(i) instanceof CTabItem) {
+			if (!((CTabItem) subControls.get(i)).isShowing())
+				continue;
+			boundsA = ((CTabItem) subControls.get(i)).getBounds();
+		}
+		for (int j = i + 1; j < subControls.size(); j++) {
+			Rectangle boundsB = null;
+			if (subControls.get(j) instanceof Control) {
+				if (!((Control) subControls.get(j)).isVisible())
+					continue;
+				boundsB = ((Control) subControls.get(j)).getBounds();
+			} else if (subControls.get(j) instanceof CTabItem) {
+				if (!((CTabItem) subControls.get(j)).isShowing())
+					continue;
+				boundsB = ((CTabItem) subControls.get(j)).getBounds();
+			}
+			assertFalse("Overlap between <" + subControls.get(i) + "> and <" + subControls.get(j) + ">\n" + boundsA
+					+ " overlaps " + boundsB, boundsA.intersects(boundsB));
+		}
+		assertEquals("Element <" + subControls.get(i) + "> outside folder.", folderBounds.intersection(boundsA),
+				boundsA);
+	}
+}
+
+private static Control[] reflection_getChildControls(CTabFolder tabFolder) {
+	String childControlArrayName = "controls";
+	try {
+		Field field = CTabFolder.class.getDeclaredField(childControlArrayName);
+		field.setAccessible(true);
+		return (Control[]) field.get(tabFolder);
+	} catch (Exception e) {
+		fail("Failed to access controls via reflections.");
+		return null;
+	}
+}
+
 }