e4 CSS engine shouldn't overwrite colors explicitly set on controls using setForeground and/or setBackground

[minduch]

Colors set on widgets foreground and/or background does not work in Dark theme in Eclipse. It works just fine in Light/Classic. If colors are changed during various events later on (e.g. when user is processing the view or dialog box) to remove, set and/or change color, it seems to suddenly work if e.g. an entry field has received focus, but this cannot be a direct widget creation. I can't really see a pattern here, apart of entry field fore- and background being able to be set once the entry field (Text widget) has received focus.

Please find attached test case plugin with SWTColors view (open it in Eclipse using Views - Show view) along with dialog box showing the same thing. Also shows that flat toolbar items hovering color in dark theme is not really working.

Tested environment: Eclipse 4.25 M3, Java Temurin x64 18.0.2.1 under Windows 11 (with dark mode for system and apps).

Seems to be related to #259?

Below colors is initialized using:

      Display display=parent.getDisplay();
      colors=new Color [] 
            {
            new Color(display,255,127,127), // 0 - Red FG
            new Color(display,127,255,127), // 1 - Green FG
            new Color(display,127,127,255), // 2 - Blue FG
            new Color(display, 95, 31, 31), // 3 - Red BG
            new Color(display, 31, 95, 31), // 4 - Green BG
            new Color(display, 31, 31, 95), // 5 - Blue BG
            new Color(display,127,127,127), // 6 - Gray BG
            };

Code creating widgets and setting colors looks like:

new Label(cnr,SWT.NONE).setText("Label:");
label=new Label(cnr,SWT.NONE);
label.setLayoutData(new GridData(SWT.FILL,SWT.CENTER,true,false));
label.setText("Label with red fore- and background");
label.setForeground(colors[0]);
label.setBackground(colors[3]);

new Label(cnr,SWT.NONE).setText("CLabel:");
CLabel clabel=new CLabel(cnr,SWT.NONE);
clabel.setLayoutData(new GridData(SWT.FILL,SWT.CENTER,true,false));
clabel.setImage(getDefaultImage());
clabel.setText("CLabel with red fore- and background");
clabel.setForeground(colors[0]);
clabel.setBackground(colors[3]);

new Label(cnr,SWT.NONE).setText("Text:");
Text text=new Text(cnr,SWT.SIMPLE|SWT.BORDER);
text.setLayoutData(new GridData(SWT.FILL,SWT.CENTER,true,false));
text.setText("Text with red fore- and background");
text.setForeground(colors[0]);
text.setBackground(colors[3]);

GridData gd=new GridData(SWT.FILL,SWT.FILL,true,true);
gd.heightHint=100;
new Label(cnr,SWT.NONE).setText("Text (area):");
text=new Text(cnr,SWT.MULTI|SWT.BORDER|SWT.WRAP|SWT.H_SCROLL|SWT.V_SCROLL);
text.setLayoutData(gd);
text.setText("Text area with red fore- and background");
text.setForeground(colors[0]);
text.setBackground(colors[3]);

new Label(cnr,SWT.NONE).setText("Combo:");
Combo combo=new Combo(cnr,SWT.DROP_DOWN);
combo.setLayoutData(new GridData(SWT.FILL,SWT.CENTER,true,false));
combo.setText("Combo with blue fore- and background");
combo.setForeground(colors[2]);
combo.setBackground(colors[5]);
for ( int ii=1; ii<4; ++ii )
  combo.add("Line "+ii);

new Label(cnr,SWT.NONE).setText("CCombo:");
CCombo ccombo=new CCombo(cnr,SWT.DROP_DOWN|SWT.BORDER);
ccombo.setLayoutData(new GridData(SWT.FILL,SWT.CENTER,true,false));
ccombo.setText("CCombo with blue fore- and background");
ccombo.setForeground(colors[2]);
ccombo.setBackground(colors[5]);
for ( int ii=1; ii<4; ++ii )
  ccombo.add("Line "+ii);

new Label(cnr,SWT.SEPARATOR|SWT.HORIZONTAL).setLayoutData(new GridData(SWT.FILL,SWT.CENTER,true,false,2,1));

new Label(cnr,SWT.NONE).setText("Button (press to open dialog box):  ");
Button button=new Button(cnr,SWT.PUSH);
button.setLayoutData(new GridData(SWT.FILL,SWT.CENTER,true,false));
button.setText("  Button with blue fore- and background  ");
button.setForeground(colors[2]);
button.setBackground(colors[5]);
button.addListener(SWT.Selection,(e)->new CustomDialog(parent.getShell()).open());
button.setFocus(); // Set focus to button.

new Label(cnr,SWT.NONE).setText("Checkbox:");
button=new Button(cnr,SWT.CHECK);
button.setText("Checkbox with green fore- and background");
button.setForeground(colors[1]);
button.setBackground(colors[4]);

new Label(cnr,SWT.NONE).setText("Radio button:");
button=new Button(cnr,SWT.RADIO);
button.setText("Radio button with green fore- and background");
button.setForeground(colors[1]);
button.setBackground(colors[4]);

new Label(cnr,SWT.SEPARATOR|SWT.HORIZONTAL).setLayoutData(new GridData(SWT.FILL,SWT.CENTER,true,false,2,1));

new Label(cnr,SWT.NONE).setText("Flat toolbar (check hovering mouse over)\nGray background");
ToolBar toolbar=new ToolBar(cnr,SWT.FLAT);
toolbar.setBackground(colors[6]); // Gray

ToolItem item=new ToolItem(toolbar,SWT.NONE);
item.setText("Red text - please check hover colors");
item.setForeground(colors[0]);
item.setBackground(colors[3]);

item=new ToolItem(toolbar,SWT.SEPARATOR);
item.setForeground(colors[2]);
item.setBackground(colors[5]);

item=new ToolItem(toolbar,SWT.DROP_DOWN);
item.setText("Blue combo");
item.setForeground(colors[2]);
item.setBackground(colors[5]);

item=new ToolItem(toolbar,SWT.PUSH);
item.setText("  Green push  ");
item.setForeground(colors[1]);
item.setBackground(colors[4]);

item=new ToolItem(toolbar,SWT.CHECK);
item.setText("Green checkbox");
item.setForeground(colors[1]);
item.setBackground(colors[4]);

item=new ToolItem(toolbar,SWT.RADIO);
item.setText("Green radio button");
item.setForeground(colors[1]);
item.setBackground(colors[4]);

item=new ToolItem(toolbar,SWT.PUSH);
item.setText(" Plain button  ");

item=new ToolItem(toolbar,SWT.DROP_DOWN);
item.setText("Plain combo");

new Label(cnr,SWT.SEPARATOR|SWT.HORIZONTAL).setLayoutData(new GridData(SWT.FILL,SWT.CENTER,true,false,2,1));

new Label(cnr,SWT.NONE).setText("Flat toolbar (check hovering mouse over)\nNo background");
toolbar=new ToolBar(cnr,SWT.FLAT);

item=new ToolItem(toolbar,SWT.NONE);
item.setText("Red text - please check hover colors");
item.setForeground(colors[0]);
item.setBackground(colors[3]);

item=new ToolItem(toolbar,SWT.SEPARATOR);
item.setForeground(colors[2]);
item.setBackground(colors[5]);

item=new ToolItem(toolbar,SWT.DROP_DOWN);
item.setText("Blue combo");
item.setForeground(colors[2]);
item.setBackground(colors[5]);

item=new ToolItem(toolbar,SWT.PUSH);
item.setText("  Green push  ");
item.setForeground(colors[1]);
item.setBackground(colors[4]);

item=new ToolItem(toolbar,SWT.CHECK);
item.setText("Green checkbox");
item.setForeground(colors[1]);
item.setBackground(colors[4]);

item=new ToolItem(toolbar,SWT.RADIO);
item.setText("Green radio button");
item.setForeground(colors[1]);
item.setBackground(colors[4]);

item=new ToolItem(toolbar,SWT.PUSH);
item.setText(" Plain button  ");

item=new ToolItem(toolbar,SWT.DROP_DOWN);
item.setText("Plain combo");

ColorsSWT.zip

[minduch]

Update - now on Eclipse 2022-09 RC1 - some colors can be made to work!

Some control colors in Dark theme work if set once again (until a View or workbench part loses focus), works better in a dialog box, see why below in 7).

Sample code as a plugin project ColorSWT containing a workbench View and a dialog box is changed and attached in ZIP file along with screen captures on Windows 11 running Eclipse Temurin 18.0.2+1 64-bit with Eclipse 2022-09 RC1.

ColorsSWT.zip

The issue description of this case should perhaps read "Eclipse e4 CSS engine shouldn't overwrite colors explicitly set on controls using setForeground and/or setBackground" instead of "Colors set on widgets foreground and/or background does not work in Dark theme in Eclipse". Please note that it could also be e.g. a gradient instead of background color on some components such as CLabel, that too doesn't work properly.

How to reproduce:

1. Add the plugin project ColorSWT to Eclipse and launch an Eclipse configuration product (e.g. the Eclipse IDE) with this ColorSWT project loaded (it only has a view located in the area of the Problems view). Make sure you have Dark theme set in that launched Eclipse IDE, otherwise change to Dark theme and relaunch it (restart doesn't always work from the theme dialog box when you debug or run from another Eclipse instance).
2. The initial colors are not working in ColorSWT view. The test case has the last toolbar button Plain button - reset+set colors again to reset colors again (press it), shown in screen capture attached in ZIP file Screenshot 2022-09-03-initial-colors.png.
3. When the toolbar button Plain button - reset+set colors again is pressed, the colors are reset using setForeground(null) and setBackground(null), then again to the color requested, it works -- until the view loses focus. Shown in screen capture Screenshot 2022-09-03-colors-reset.png in attached ZIP file.
4. When focus is lost, only the Toolbar Items has colors that remains. Shown in screen capture Screenshot 2022-09-03-colors-reset-focus-lost.png in attached ZIP file.
5. Open dialog box using the button Button (press to open dialog box). The dialog box initial colors are not working, see screen capture Screenshot 2022-09-03-dialog-initial.png in attached ZIP file.
6. Dialog box after pressing Plain button - reset+set colors again toolbar item makes colors work, see screen capture Screenshot 2022-09-03-dialog-colors-reset.png in attached ZIP file.
7. If dialog box loses focus, it does not change its colors, and this is understandable as the Eclipse View changes its CSS colors for a view not in focus (and the dialog box does not).

Eclipse is now updated to 2022-09 RC1 and still no difference with any 2022-09 M1-M2 or 2022-06 (and perhaps also eralier).

[mickaelistria]

Thanks, I've retitled according to your feedback. It seems relatively easy for the e4 CSS theme engine to verify that getForeground/getBackground do not return the default Color before calling the related setters. Would you be willing to submit a patch?

[vogella]

IIRC we already have a key for widgets which should not be styled by the CSS egine. Would that help @minduch? If we do not style widgets by default if their colors are set we would most likely break all themes out there.

[minduch]

Yes, please get me that info fast, we’re running out of time. On 6 Sep 2022, at 09:31, Lars Vogel @.***> wrote:



IIRC we already have a key for widgets which should not be styled by the CSS egine. Would that help @minduchhttps://github.com/minduch? If we do not style widgets by default if their colors are set we would most likely break all themes out there.

— Reply to this email directly, view it on GitHubhttps://github.com/eclipse-platform/eclipse.platform.ui/issues/306#issuecomment-1237765289, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AGXD5VVMHLHO2RPLCNGICPTV43XLHANCNFSM6AAAAAAQCJHNDY. You are receiving this because you were mentioned.Message ID: @.***>

[vogella]

IIRC we already have a key for widgets which should not be styled by the CSS egine. Would that help @minduch? If we do not style widgets by default if their colors are set we would most likely break all themes out there.

Sorry doing a quick search did not show such a key. I'm not sure I understand the problem you are trying to solve. You could assign a CSS class or ID to your widgets and also style them also via CSS https://www.vogella.com/tutorials/Eclipse4CSS/article.html#styling-based-on-identifiers-and-classes

[mickaelistria]

I think it can be very valid that in some cases, the colors of widgets cannot be statically determined (eg you want your widget to follow some color scheme of an external dynamic image to better identify how each widgets maps which part of the image). The theme should really be lower priority than developer calling setForeground/setBackground in their code. If the CSS engine can detect such cases, then it should skip overriding colors for those.

[minduch]

But wouldn't just getForeground() and getBackground() returning "null" do the trick? Then if CSS applies another color, that color is stored in the widgets data for comparison in case theme changes, i.e. when getForeground() getBackground() doesn't return null. Then some special cases for widgets supporting setBackground with e.g. gradients such as CLabel.

I don't know how fonts are handled though. I haven't tried that out, and the CSS engine doesn't seem to break that (at least not in our case).

[minduch]

Can somebody point me in the direction that seems to be Windows-specific and another scope: Windows Handles used by SWT? We run out of them all the time (SWT Error and we only consume/release as one should by the book). But MS Edge is a very likely source to the problem.

[minduch]

Thanks, I've retitled according to your feedback. It seems relatively easy for the e4 CSS theme engine to verify that getForeground/getBackground do not return the default Color before calling the related setters. Would you be willing to submit a patch?

@mickaelistria How do I best do this? The Eclipse environment is pretty large and there are so many versions... And from what I have read/understood, you have to become a committer first, no?

[mickaelistria]

How do I best do this?

There is no "best" because the optimal workflow can slightly vary between developers, but here is what is the current recommendation to get started contributing: https://github.com/eclipse-platform/.github/blob/main/CONTRIBUTING.md#create-an-eclipse-development-environment

The Eclipse environment is pretty large and there are so many versions...

Versions do not really matter as the Eclipse Platform project merges patches only in the master branch (so patches need to target current master). As for large, it is indeed, so here is a hint about where such change could be implemented: look for all setForeground/setBackground in https://github.com/eclipse-platform/eclipse.platform.ui/tree/master/bundles/org.eclipse.e4.ui.css.swt

And from what I have read/understood, you have to become a committer first, no?

Not al all, anyone can submit a patch! Eclipse Platform is a real community open-source project and patches are reviewed independently of who authored them. Committers are elected developers who have the ability to merge patches directly (and thus the duty of reviewing), but in every release, a lot of patches for non-committers are happily merged on master and made available in next release.

[vogella]

https://www.vogella.com/tutorials/EclipsePlatformDevelopment/article.html gives a description how to contribute. I updated it today the latest change should be available in 20 min but you could already start at the top of the article and go through it.

[merks]

There are details here too:

https://github.com/eclipse-platform/.github/blob/main/CONTRIBUTING.md

including an automated setup process to augment the very useful information in Lars' tutoral...

You can set up a pre-configured IDE for development of the Eclipse SDK using the following link.

[tomaswolf]

@vogella wrote:

IIRC we already have a key for widgets which should not be styled by the CSS egine. Would that help @minduch? If we do not style widgets by default if their colors are set we would most likely break all themes out there.

Sorry doing a quick search did not show such a key.

Probably you were thinking of org.eclipse.e4.ui.css.disabled. See EGit's UIUtils class :-) and also see bug 559321.

[minduch]

Sure. In the past I haven’t done it, but I have over 20 years of Java coding and Eclipse, and over 45 years of programming architecture design communication UI product development, so if you tell me how to do it, I’ll give it a shot. You probably have docs about it, and I have also reported quite a few issues in Eclipse and Jetty over time. I would perhaps need a bit of guidance though. On 6 Sep 2022, at 08:54, Mickael Istria @.***> wrote:



Thanks, I've retitled according to your feedback. It seems relatively easy for the e4 CSS theme engine to verify that getForeground/getBackground do not return the default Color before calling the related setters. Would you be willing to submit a patch?

— Reply to this email directly, view it on GitHubhttps://github.com/eclipse-platform/eclipse.platform.ui/issues/306#issuecomment-1237732119, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AGXD5VS7BR64BIRAKP6D3FDV43TDVANCNFSM6AAAAAAQCJHNDY. You are receiving this because you authored the thread.Message ID: @.***>

[mickaelistria]

You can look at https://github.com/eclipse-platform/.github/blob/main/CONTRIBUTING.md#create-an-eclipse-development-environment . Once the environment is ready, you can start hacking the necessary code (most likely somewhere in https://github.com/eclipse-platform/eclipse.platform.ui/tree/master/bundles/org.eclipse.e4.ui.css.swt/src/org/eclipse/e4/ui ). You can easily try "Debug As > Eclipse application" on this plugin to debug it efficiently and analyze the effect of your changes. Adding a test case for that seems relatively easy, so you may want to start by trying to write such a test that will accelerate your development and prevent from further regressions. You can contribute such test somewhere inside https://github.com/eclipse-platform/eclipse.platform.ui/tree/master/tests/org.eclipse.e4.ui.tests.css.swt/src/org/eclipse/e4/ui . Good luck!

[minduch]

I'm sorry, right now I do not have the opportunity to do any committing for Eclipse, I mearely wished to notify you guys about that this is still an issue in Eclipse 2023-06 M2 v4.28.

[minduch]

Also note new error in Eclipse 2023-06 M2 v4.28 '/css/gef_dark problem included in problem #628 following this problem with missing /css/dark.css.

[jukzi]

@minduch please don't duplicate issues. Also please note that the community needs your help to fix bugs.

[minduch]

I am currently admitted to the hospital for a stroke, so I will not be able to help for quite a while, I’m sorry.

Christopher Mindus

CTO & Founder

Christopher Mindus

CTO & Founder

[Image.jpeg]

1 Rue du Gabian•98000 Monaco•Monaco

Mob+33 618 614 089•Tel+377 99 90 32 66

@.**@.>•www.mindus.cohttp://www.mindus.co/

1 Rue du Gabian • 98000 Monaco • Monaco

Mob +33 618 614 089 • Tel +377 99 90 32 66

@.**@.> • www.mindus.cohttp://www.mindus.co/

---

From: Jörg Kubitz @.> Sent: Monday, May 22, 2023 8:43:02 AM To: eclipse-platform/eclipse.platform.ui @.> Cc: Christopher Mindus @.>; Mention @.> Subject: Re: [eclipse-platform/eclipse.platform.ui] e4 CSS engine shouldn't overwrite colors explicitly set on controls using setForeground and/or setBackground (Issue #306)

@minduchhttps://github.com/minduch please don't duplicate issues. Also please note that the community needs your help to fix bugs.

— Reply to this email directly, view it on GitHubhttps://github.com/eclipse-platform/eclipse.platform.ui/issues/306#issuecomment-1556627504, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AGXD5VSSDFCGOQXTSGY2AZDXHMDHNANCNFSM6AAAAAAQCJHNDY. You are receiving this because you were mentioned.Message ID: @.***>

[jukzi]

Best wishes for a speedy recovery!

[minduch]

Update for this case: now with version Eclipse 4.31 M3 on Windows 11.

For Eclipse Dark Theme, I found a cute workaround without fiddling into Eclipse code. Every widget must be extended and have the following code added:

  // CSS and custom colors.
  private Color cf,cb,f,b;

  // Avoid exception from subclassed widget.
  @Override protected void checkSubclass() {}

  @Override public void setForeground(Color color)
    {
    // Save color as CSS or custom.
    boolean css=StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE).getCallerClass().getName().equals("org.eclipse.e4.ui.css.swt.helpers.CSSSWTColorHelper");
    if ( css )
      cf=color;
    else
      f=color;

    if ( css )
      {
      // CSS sets color: keep custom color if any.
      if ( f!=null )
        return;
      }
    else
      {
      // Custom: if color is reset, use CSS color.
      if ( color==null )
        color=cf;
      }

    super.setForeground(color);
    }

  @Override public void setBackground(Color color)
    {
    // Save color as CSS or custom.
    boolean css=StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE).getCallerClass().getName().equals("org.eclipse.e4.ui.css.swt.helpers.CSSSWTColorHelper");
    if ( css )
      cb=color;
    else
      b=color;

    if ( css )
      {
      // CSS sets color: keep custom color if any.
      if ( b!=null )
        return;
      }
    else
      {
      // Custom: if color is reset, use CSS color.
      if ( color==null )
        color=cb;
      }

    super.setBackground(color);
    }

This works for Button, CLabel, Combo, Label, List and Text. For CCombo, the entire class must be replaced due to the package-private components Text, Button and List.

Find the attached ZIP file with the initial case re-written to make the colors work by using subclassed widgets named IZ_nnn. Also find the class CCombo as IZ_CCombo copied from Eclipse code and using IZ_Text, IZ_Button and IZ_List to handle colors correctly.

I didn't manage to do a fix for the ToolBar and ToolItem's. The button in the test case "Plain button - reset+set colors again" makes the colors almost correct for the ToolItems.

ColorsSWT.zip

This at least shows that CSS overwrites the colors set by user code at any time. I don't have enough knowledge and time to fix the CSS engine in the Eclipse code, but this should at least be a nice hint to how it can be fixed. Perhaps in the widgets themselves?

PS: There is a slight error in the code for IZ_List: the class extends Text and should be List (of course). I had zipped the source code prior to making this change.

[Veroni23]

Thanks for sharing! However, it is not exactly good programming practice to extend every single widget... Rather, it should be fixed in the base class.