Changed CSS background handling for ToolItem + CTabFolder eclipse-pla…

…tform#536 - removed CSS background handling for ToolItem + changed tests (temporary as long as there are conflicts) - handled CTabFolder
de-jcup · Feb 4, 2023 · 1693cb7 · 1693cb7
1 parent 71de89c
commit 1693cb7
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 29 deletions.
diff --git a/...ss.swt/src/org/eclipse/e4/ui/css/swt/properties/css2/CSSPropertyBackgroundSWTHandler.java b/...ss.swt/src/org/eclipse/e4/ui/css/swt/properties/css2/CSSPropertyBackgroundSWTHandler.java
@@ -36,8 +36,8 @@
 public class CSSPropertyBackgroundSWTHandler extends AbstractCSSPropertyBackgroundHandler {
 
 	@Override
-	public boolean applyCSSProperty(Object element, String property,
-			CSSValue value, String pseudo, CSSEngine engine) throws Exception {
+	public boolean applyCSSProperty(Object element, String property, CSSValue value, String pseudo, CSSEngine engine)
+			throws Exception {
 		Widget widget = SWTElementHelpers.getWidget(element);
 		if (widget != null) {
 			return super.applyCSSProperty(element, property, value, pseudo, engine);
@@ -46,8 +46,8 @@ public boolean applyCSSProperty(Object element, String property,
 	}
 
 	@Override
-	public String retrieveCSSProperty(Object element, String property,
-			String pseudo, CSSEngine engine) throws Exception {
+	public String retrieveCSSProperty(Object element, String property, String pseudo, CSSEngine engine)
+			throws Exception {
 		Widget widget = SWTElementHelpers.getWidget(element);
 		if (widget != null) {
 			return super.retrieveCSSProperty(widget, property, pseudo, engine);
@@ -56,13 +56,13 @@ public String retrieveCSSProperty(Object element, String property,
 	}
 
 	@Override
-	public void applyCSSPropertyBackgroundColor(Object element, CSSValue value,
-			String pseudo, CSSEngine engine) throws Exception {
+	public void applyCSSPropertyBackgroundColor(Object element, CSSValue value, String pseudo, CSSEngine engine)
+			throws Exception {
+
 		Widget widget = (Widget) ((WidgetElement) element).getNativeWidget();
 		switch (value.getCssValueType()) {
 		case CSSValue.CSS_PRIMITIVE_VALUE:
-			Color newColor = (Color) engine.convert(value, Color.class, widget
-					.getDisplay());
+			Color newColor = (Color) engine.convert(value, Color.class, widget.getDisplay());
 			if (widget instanceof CTabItem) {
 				CTabFolder folder = ((CTabItem) widget).getParent();
 				if ("selected".equals(pseudo)) {
@@ -72,24 +72,34 @@ public void applyCSSPropertyBackgroundColor(Object element, CSSValue value,
 					CSSSWTColorHelper.setBackground(folder, newColor);
 				}
 			} else if (widget instanceof ToolItem) {
-				// ToolItem prevents itself from repaints if the same color is set
-				((ToolItem) widget).setBackground(newColor);
+				// In this case we (currently) just do nothing for CSS.
+				// Otherwise there is a conflict with former CTabItem handling - the
+				// toggle buttons are not highlighted any more in dark theme.
+				// This is only a temporary workaround.
+				// (see #467)
 			} else if (widget instanceof Control) {
 				GradientBackgroundListener.remove((Control) widget);
-				CSSSWTColorHelper.setBackground((Control) widget, newColor);
-				CompositeElement.setBackgroundOverriddenByCSSMarker(widget);
+
+				if (widget instanceof CTabFolder) {
+					// in this case we just do nothing for CSS
+					// Reason: Otherwise there are conflict with former CTabItem handling
+					// CompositeElement.setBackgroundOverriddenByCSSMarker(widget) does
+					// also assume that CTabFolder does no background overrides by CSS.
+					// This avoids color changes on tab menu opening (see #536).
+				} else {
+					CSSSWTColorHelper.setBackground((Control) widget, newColor);
+					CompositeElement.setBackgroundOverriddenByCSSMarker(widget);
+				}
 			}
 			break;
 		case CSSValue.CSS_VALUE_LIST:
-			Gradient grad = (Gradient) engine.convert(value, Gradient.class,
-					widget.getDisplay());
+			Gradient grad = (Gradient) engine.convert(value, Gradient.class, widget.getDisplay());
 			if (grad == null) {
 				return; // warn?
 			}
 			if (widget instanceof CTabItem) {
 				CTabFolder folder = ((CTabItem) widget).getParent();
-				Color[] colors = CSSSWTColorHelper.getSWTColors(grad,
-						folder.getDisplay(), engine);
+				Color[] colors = CSSSWTColorHelper.getSWTColors(grad, folder.getDisplay(), engine);
 				int[] percents = CSSSWTColorHelper.getPercents(grad);
 
 				if ("selected".equals(pseudo)) {
@@ -109,12 +119,11 @@ public void applyCSSPropertyBackgroundColor(Object element, CSSValue value,
 	}
 
 	@Override
-	public void applyCSSPropertyBackgroundImage(Object element, CSSValue value,
-			String pseudo, CSSEngine engine) throws Exception {
+	public void applyCSSPropertyBackgroundImage(Object element, CSSValue value, String pseudo, CSSEngine engine)
+			throws Exception {
 		// Widget control = (Widget) element;
 		Widget widget = (Widget) ((WidgetElement) element).getNativeWidget();
-		Image image = (Image) engine.convert(value, Image.class,
-				widget.getDisplay());
+		Image image = (Image) engine.convert(value, Image.class, widget.getDisplay());
 		if (widget instanceof CTabFolder && "selected".equals(pseudo)) {
 			((CTabFolder) widget).setSelectionBackground(image);
 		} else if (widget instanceof Button) {
@@ -129,20 +138,17 @@ public void applyCSSPropertyBackgroundImage(Object element, CSSValue value,
 	}
 
 	@Override
-	public String retrieveCSSPropertyBackgroundColor(Object element,
-			String pseudo, CSSEngine engine) throws Exception {
+	public String retrieveCSSPropertyBackgroundColor(Object element, String pseudo, CSSEngine engine) throws Exception {
 		Widget widget = (Widget) element;
 		Color color = null;
 		if (widget instanceof CTabItem) {
 			if ("selected".equals(pseudo)) {
-				color = ((CTabItem) widget).getParent()
-						.getSelectionBackground();
+				color = ((CTabItem) widget).getParent().getSelectionBackground();
 			} else {
 				color = ((CTabItem) widget).getParent().getBackground();
 			}
 
-		}
-		else if (widget instanceof ToolItem) {
+		} else if (widget instanceof ToolItem) {
 			color = ((ToolItem) widget).getBackground();
 		}
 
@@ -152,5 +158,4 @@ else if (widget instanceof Control) {
 		return engine.convert(color, Color.class, null);
 	}
 
-
 }
diff --git a/bundles/org.eclipse.ui.themes/css/dark/e4-dark_globalstyle.css b/bundles/org.eclipse.ui.themes/css/dark/e4-dark_globalstyle.css
@@ -57,9 +57,9 @@ TabFolder > Composite > * > * { /* [style~='SWT.NO_BACKGROUND'] <- generate E4 n
 	color:'#org-eclipse-ui-workbench-DARK_FOREGROUND'; 
 }
 
+/* Toolbar should inherit the colors of its container to avoid drawing artifacts*/
 ToolBar {
-    /* Because resizing leads to a change in style inheritance we do not use 'inherit', but define an explicit background color here */
-    background-color: '#org-eclipse-ui-workbench-DARK_BACKGROUND';
+	background-color:inherit;	
 }
 
 Combo,

diff --git a/tests/org.eclipse.e4.ui.tests.css.swt/src/org/eclipse/e4/ui/tests/css/swt/ToolItemTest.java b/tests/org.eclipse.e4.ui.tests.css.swt/src/org/eclipse/e4/ui/tests/css/swt/ToolItemTest.java
@@ -51,6 +51,7 @@ protected ToolItem createTestToolItem(String styleSheet, int styleBit) {
 	}
 
 	@Test
+	@Disabled("Implementation was removed because problems when parent toolbar inside CTabFolder")
 	void testBackgroundColor() {
 		var toolItemToTest = createTestToolItem("ToolItem { background-color: #FF0000;}",
 				SWT.PUSH);
@@ -75,6 +76,7 @@ void testSelectedPseudo() {
 	}
 
 	@Test
+	@Disabled("Implementation was removed because problems when parent toolbar inside CTabFolder")
 	void ensurePseudoAttributeAllowsToSelectionPushButton() {
 		var toolItemToTest = createTestToolItem(
 				"ToolItem[style~='SWT.CHECK'] { background-color: #FF0000; color: #0000FF }", SWT.CHECK);
@@ -89,4 +91,18 @@ void ensurePseudoAttributeAllowsToSelectionPushButton() {
 		assertNotEquals(BLUE, unStyledBToolItem.getForeground().getRGB());
 
 	}
+
+	@Test
+	void ensurePseudoAttributeAllowsToSelectionPushButtonOnlyForeground() {
+		var toolItemToTest = createTestToolItem(
+				"ToolItem[style~='SWT.CHECK'] { color: #0000FF }", SWT.CHECK);
+
+		assertEquals(BLUE, toolItemToTest.getForeground().getRGB());
+
+		var unStyledBToolItem = createTestToolItem(
+				"ToolItem[style~='SWT.PUSH'] { color: #0000FF }", SWT.CHECK);
+
+		assertNotEquals(BLUE, unStyledBToolItem.getForeground().getRGB());
+
+	}
 }