Community
Participate
Working Groups
Build Identifier: On Ubuntu, using a TableViewer with a StyledCellLabelProvider gives an incorrect text color for selected table items when the table does not have focus. This problem only seems to affect the default Ambiance/Radiance theme on Ubuntu, not other themes. Reproducible: Always Steps to Reproduce: Launch snippet.
Created attachment 205494 [details] Snippet
Created attachment 205498 [details] Screenshot showing inconsistent foreground colors.
Ubuntu 11.10 This is an SWT problem that can also be reproduced with Snippet236. To make it more visible, comment out the line "textLayout.setStyle(style1, ...". For selected items, the text "Standard" should then be rendered in black when the table doesn't have focus. Setting to major, since this is visible all over the SDK and makes selected items hard to read (e.g. in the Open Type and Quick Outline dialogs).
This is the SWT code that draws the background: int /*long*/ style = OS.gtk_widget_get_style (widget); //TODO - parity and sorted byte[] detail = Converter.wcsToMbcs (null, "cell_odd", true); OS.gtk_paint_flat_box (style, window, OS.GTK_STATE_SELECTED, OS.GTK_SHADOW_NONE, rect, widget, detail, rect.x, rect.y, rect.width, rect.height); http://fossies.org/dox/gtk+-2.24.10/gtkstyle_8c_source.html#l3644 says about the implementation: 3673 /* This has to be really broken; alex made me do it. -jrb */ 3674 if (widget && gtk_widget_has_focus (widget)) 3675 gc1 = style->base_gc[state_type]; 3676 else 3677 gc1 = style->base_gc[GTK_STATE_ACTIVE]; The problem is that gtk_paint_flat_box adjusts the background color dynamically behind the scenes. Obviously, clients that use this function have no clue, and when they draw text with a static fg color, then we see the observed mismatch. The fix is either to make sure we use a matching fg color when rendering text, or to stop supporting the inactive selection bg color (like many apps, e.g. Nautilus, System Settings, gtk3-demo, ...).
Created attachment 220859 [details] Patch for R3_8_maintenance This patch implements the first solution (keep rendering an inactive selection differently). The changes in the Widget class are necessary to support Tables/Trees with a non-null background/foreground color. E.g. JDT's Quick Outline Ctrl+O or add this to the snippet: tv.getTable().setBackground(display.getSystemColor(SWT.COLOR_INFO_BACKGROUND)); tv.getTable().setForeground(display.getSystemColor(SWT.COLOR_INFO_FOREGROUND)); For normal StyledCellLabelProviders, fixing the foreground color would be enough. But if you change the snippet like this, you see that the background is also necessary for a complete solution: col2.setLabelProvider(new StyledCellLabelProvider() { @Override protected void paint(Event event, Object element) { super.paint(event, element); Rectangle b = ((TableItem) event.item).getBounds(1); b.width /= 2; b.x = b.x + b.width; b.height -= 8; b.y += 4; event.gc.fillRectangle(b); event.gc.drawRectangle(b); } ...
Is there a chance that we will get the suggested fix committed for SR2?
I should soon be an SWT committer, and this is one of the first fixes I'll commit (unless somebody commits it even before). After it's been verified in an I-build, I agree it should also go into SR2.
Committed the patch with a few tweaks to master: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=de2297113fcb0174b02c429f9d73e7ee7a1eccda
Version: 4.3.0 Build id: N20121018-2000 That solves the problem for me :-) Can this fix be backported for 3.8.2? Thank You!
(In reply to comment #8) > Committed the patch with a few tweaks to master: > http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=de2297113fcb0174b02c429f9d73e7ee7a1eccda Silenio, OK to release for 3.8.2 and 4.2.2? The commit should be cherry-pickable without problems.
(In reply to comment #10) > (In reply to comment #8) > > Committed the patch with a few tweaks to master: > > http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=de2297113fcb0174b02c429f9d73e7ee7a1eccda > > Silenio, OK to release for 3.8.2 and 4.2.2? > The commit should be cherry-pickable without problems. The changes in Display,Tree and Table seem fine, but the changes in Widget are affecting other widgets and that does not look good. If you run the example below and press/hold the mouse on the button (depressed state), the button foreground is black instead of red. import org.eclipse.swt.*; import org.eclipse.swt.widgets.*; import org.eclipse.swt.layout.*; public class TestButton { public static void main(String[] args) { Display display = new Display(); Shell shell = new Shell(display); shell.setLayout(new GridLayout(1, false)); { Button button = new Button(shell, SWT.NONE); button.setText("Hello World1"); button.setForeground(display.getSystemColor(SWT.COLOR_RED)); } { Button button = new Button(shell, SWT.NONE); button.setText("Hello World2"); button.setForeground(display.getSystemColor(SWT.COLOR_RED)); } shell.pack(); shell.open(); while (!shell.isDisposed()) { if (!display.readAndDispatch()) display.sleep(); } display.dispose(); } }
(In reply to comment #11) Sorry about that. I did look for such problems, but didn't find one. Fixed in master with http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=4d7c12dd6a4904cb398f87337e928a23b40052cc (In reply to comment #4) This is the actual source: http://git.gnome.org/browse/gtk+/tree/gtk/gtkstyle.c?h=gtk-2-24#n3673
I released a wrong fix for Tree and I forgot List, which suffers from the same problem if you set a custom foreground color. Fixed with: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=c6e8d03bf2c357cdfc60ecf9035affacefc3b025 Then I hacked Tab.java from the ControlExample like this: void setExampleWidgetBackground () { if (colorAndFontTable == null) return; // user cannot change color/font Control [] controls = getExampleControls (); - if (!instance.startup) { + if (true) { + backgroundColor = display.getSystemColor(SWT.COLOR_INFO_BACKGROUND); .. and did the same for setExampleWidgetForeground (). Observation: We have many more controls that need the void setForegroundColor (GdkColor color) { setForegroundColor (handle, color, false); } Combo, DateTime, Spinner, and Text all have the same problem if text is selected and then the user gives focus to another control. Inactive TabItems are also hard to read. => I would use "setForegroundColor (handle, color, false);" for all these. Buttons are also hard to read when you hover them. And it's arguable whether their foreground color should really be used when they are selected, hovered, or pressed down. In all those states, the client doesn't have control over the background color, so you can always have conflicts depending on the color theme. => Neither solution is completely satisfactory, so I would leave Button as is.
(In reply to comment #13) > Combo, DateTime, Spinner, and Text all have the same problem if text is > selected and then the user gives focus to another control. Inactive TabItems > are also hard to read. > => I would use "setForegroundColor (handle, color, false);" for all these. Fixed with http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=b557cbf501fa8ef82685fc23f3e296ddd14da4a9 Let's run this through the M3 test pass next week and then backport.
Created attachment 223360 [details] Patch for R3_8_maintenance Here's the patch for R3_8_maintenance and R4_2_maintenance that combines all 4 commits from master. Manual changes w.r.t. the original commits: - in Widget, resolved long /*int*/ -> int /*long*/ conflicts - in Table and Tree, replaced references to Widget's gtk_widget_has_focus(handle) with the previously used OS.GTK_WIDGET_HAS_FOCUS(handle)
+1
Fixed in 4.2.2/3.8.2: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=R3_8_maintenance&id=94d63723dc23df664a35544d15de2808ae83933f http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=R4_2_maintenance&id=2d2c1ca51aae3ca5a185e7eb22b776664ead4939
Verified in M20130116-1800 and M20130116-1030.
