| Summary: | [Themes] Background changes to dark during the startup splash screen | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Stefan Winkler <stefan> | ||||||||
| Component: | UI | Assignee: | Lars Vogel <Lars.Vogel> | ||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||
| Severity: | normal | ||||||||||
| Priority: | P3 | CC: | andrea.guarinoni, daniel.rolka, kaloyan, Lars.Vogel, matthias.mailaender, sudol.wojciech | ||||||||
| Version: | 4.4 | ||||||||||
| Target Milestone: | 4.5 M5 | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Whiteboard: | |||||||||||
| Bug Depends on: | 419377, 430208, 431742 | ||||||||||
| Bug Blocks: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Stefan Winkler
I have started to play around with the CSS.
The respective CSS should be something like
BasicSplashHandler$AbsolutePositionProgressMonitorPart Label {
opacity: 0.0; /* is opacity supported in SWT/CSS? */
color: #9c9696;
}
But the selector does not seem to work. Neither do
BasicSplashHandler$AbsolutePositionProgressMonitorPart
BasicSplashHandler$AbsolutePositionProgressMonitorPart > Label
BasicSplashHandler$AbsolutePositionProgressMonitorPart *
So I'm a bit stuck here on how this could be fixed.
Reproduced in Windows 7. This cannot be fixed with only CSS. Currently: - every widget selector that contains a '$' in its name, it's not a valid selector; - opacity, rgba, or any kind of assignement of a transparent color is not supported. (In reply to Stefan Winkler from comment #1) > I have started to play around with the CSS. > The respective CSS should be something like > > BasicSplashHandler$AbsolutePositionProgressMonitorPart Label { > opacity: 0.0; /* is opacity supported in SWT/CSS? */ > color: #9c9696; > } > > But the selector does not seem to work. Neither do > > BasicSplashHandler$AbsolutePositionProgressMonitorPart > BasicSplashHandler$AbsolutePositionProgressMonitorPart > Label > BasicSplashHandler$AbsolutePositionProgressMonitorPart * > > So I'm a bit stuck here on how this could be fixed. Even if we fix Bug 431742, I suggest to add CSS ID to the widgets on the splash screen so that they can easily be styled. See https://wiki.eclipse.org/Eclipse4/RCP/CSS#Setting_the_Widget_Class_and_Id @Stefan, can you provide a patch which sets the ID for the widges of BasicSplashHandler? I have added IDs to the widgets of BasicSplashHandler and added suitable CSS rules. Changeset is here: https://git.eclipse.org/r/#/c/24309/ One problem still remained: I did not find a way to make (leave) the background of the label as it was. As a workaround, I added a special handler for "background-color: inherit;", whÃch was so far not supported. The patch now does what's intended, namely keep the Splash screen colors even when the dark theme is activated. But handling 'inherit' directly in org.eclipse.e4.ui.css.swt.properties.css2.CSSPropertyBackgroundSWTHandler.applyCSSPropertyBackgroundColor(Object, CSSValue, String, CSSEngine) seems a bit 'dirty-hacky', so maybe someone has a better suggestion? Thanks Stefan, fixed with https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=29bb58b693e30bc43d9847eb42a2a82182d34ba1 The progress bar of the splash screen still goes to dark in Build I20140427-2030 if the last theme was the dark theme. Stefen, can you provide a fix for this, even without the support of the inherit attibute? (In reply to Lars Vogel from comment #7) > The progress bar of the splash screen still goes to dark in Build > I20140427-2030 if the last theme was the dark theme. > > Stefen, can you provide a fix for this, even without the support of the > inherit attibute? Hi Lars, the progress bars seem to be very platform-specific. Most platforms seem to ignore foreground color settings and some even ignore background colors. Which platform did you observe this on? Only the background or the foreground as well? I have created a patch here https://git.eclipse.org/r/25919 can you try? (In reply to Stefan Winkler from comment #8) > I have created a patch here > https://git.eclipse.org/r/25919 > can you try? Thanks. I use Ubuntu 14.04 Unfortunately I'm unable to reproduce this issue from a runtime configuration, only from a Eclipse SDK build. Daniel / Stefan any ideas how to test this without committing it? I attach a screenshot of my splash Created attachment 242722 [details]
Spashscreen still incorrect
(In reply to Lars Vogel from comment #10) > Created attachment 242722 [details] > Spashscreen still incorrect I think we should consider the transparent background for the labels on the splash screen. If setting the proper color values to the null in the splash screen related classes doesn't work, we can use the custom 'onPaint' event listeners. Daniel (In reply to Daniel Rolka from comment #11) > (In reply to Lars Vogel from comment #10) > > Created attachment 242722 [details] > > Spashscreen still incorrect > > I think we should consider the transparent background for the labels on the > splash screen. If setting the proper color values to the null in the splash > screen related classes doesn't work, we can use the custom 'onPaint' event > listeners. > > Daniel I think Stefan also posted a patch to support the inherit attribute so that widgets could inherit the background color of their parents. Stefan can you like the related bug to this one? (In reply to Daniel Rolka from comment #11) > I think we should consider the transparent background for the labels on the > splash screen. If setting the proper color values to the null in the splash > screen related classes doesn't work, we can use the custom 'onPaint' event > listeners. The background is set to null per default. The issue is that when the CSS engine and theme are loaded, the background is applied based on the theme and there is currently no way to specify a null background in themes. I have submitted patches to two bugs: Bug 419377 - [CSS] Setting a property to 'inherit' fires a IllegalStateException This would make it possible to set a CSS property to the value 'inherit'. The CSS engine would look for the widget's parent and get the background value from that and apply it to the current widget. This would not solve this problem entirely, however, because "control.setBackground(null); control.getBackground();" does not evaluate to null in SWT, but to the default background. :-( Bug 430208 - [CSS] Add support for RGBA or 'transparent' color This would make it possible to set the CSS background-color property to the value 'transparent' which will cause the CSS engine to call control.setBackground(null). Of course, this only works for Label and Button. The problem with that bug is the getting the property can not return "transparent" for getting the CSS style, because (see above) I cannot check if the background is null without using reflection. So, both bugs are currently on hold, more or less, because they would require changes in SWT. Any advice how I can see the splash screen and the progress indicator for a runtime IDE? I was able to change the background of the progress label to transparent using rgba(). Successfully tested with Eclipse 4.4.1 on Ubuntu 14.04 and Windows 7. Here is the patch: https://git.eclipse.org/r/37695 Please, consider this for 4.4.2 Luna SR2. The splash screen of my product looks quite ugly when the dark theme is used. I don't find a way to override this color setting in another CSS file, so I have to wait for the fix in the Platform. (In reply to Kaloyan Raev from comment #15) > I was able to change the background of the progress label to transparent > using rgba(). Successfully tested with Eclipse 4.4.1 on Ubuntu 14.04 and > Windows 7. > > Here is the patch: https://git.eclipse.org/r/37695 Can you describe your test setup? If I apply your patch under Ubuntu and start a runtime IDE (where the last styling was dark, I still get an ugly splash. My setup was Zend Studio 12 based on Eclipse 4.4.1. I now tried with the Eclipse PHP EPP package and I have the successful result. I edit the plugins/org.eclipse.ui.themes_1.0.1.v20140819-1717/css/dark/e4-dark_globalstyle.css and then restart the IDE. What do you mean by "runtime IDE"? (In reply to Kaloyan Raev from comment #17) > What do you mean by "runtime IDE"? I typically test patches for the IDE by starting a runtime IDE from my current IDE. See here for an example http://www.vogella.com/tutorials/EclipsePlugIn/article.html#firstplugin_start Oh, yes. This worked too. This is what I did before pushing to Gerrit. I am almost certain that we do not support rbga(). For these reasons: - rgba() is CSS3 and AFAIK, our implementation is based on CSS2 right now - in the code, we have RGBColorImpl (org.eclipse.e4.ui.css.core.impl.dom.RGBColorImpl) which parses rgb() colors, but there is no trace of rgba() parsing there. - Even if we could parse rgba(), I do not know of a possibility to specify colors with an alpha channel (or in short: RGBA Colors) in SWT. Maybe what happens in your case, Kaloyan, is that the rgba() is not parsed correctly and therefore no color is set. Could you verify this by trying a different rgba color (such as semi-transparent red). And can you also try to specify some non-color (such as footer) instead of "rgba(0,0,0,0)". If I am correct, then this should have the same effect for you ... (In reply to Stefan Winkler from comment #20) > I am almost certain that we do not support rbga(). Could we use your new inherit attribute for Eclipse 4.5? (In reply to Lars Vogel from comment #21) > (In reply to Stefan Winkler from comment #20) > > I am almost certain that we do not support rbga(). > > Could we use your new inherit attribute for Eclipse 4.5? Yes, actually, as far as I can remember, this is exactly why I implemented the inherit support ;-) Here's the Gerrit review: https://git.eclipse.org/r/#/c/37704/ Verified on Mac. Maybe someone can give feedback if it works on windows and linux as well? (In reply to Stefan Winkler from comment #20) > Maybe what happens in your case, Kaloyan, is that the rgba() is not parsed > correctly and therefore no color is set. Could you verify this by trying a > different rgba color (such as semi-transparent red). And can you also try to > specify some non-color (such as footer) instead of "rgba(0,0,0,0)". If I am > correct, then this should have the same effect for you ... Yes, it seems you are right. Using rgba() always results in transparent background regardless of the actual values given. If I use "footer" then the background is black. Nevertheless, if rgba(0,0,0,0) does the desired result why don't we just use it? When rgba() becomes correctly supported, it will still work the same way without any change required. (In reply to Stefan Winkler from comment #22) > Here's the Gerrit review: https://git.eclipse.org/r/#/c/37704/ Thanks merged with https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=e35bc53b11f123d444b6c6f86e59a54562dbbfcf I retest this with the integration build tomorrow. Created attachment 249265 [details]
Screenshot
I still get a black label on Build id: I20141208-2000 under Linux. Stefan does you patch work on Mac OS with the latest integration build?
. Lars, Stefan's patch should not affect the Build ID label. It should affect only the progress label - the one saying "Loading <plugin-id>". Perhaps, there is something in your testing environment that affects the Build ID label. (In reply to Kaloyan Raev from comment #27) > Lars, Stefan's patch should not affect the Build ID label. It should affect > only the progress label - the one saying "Loading <plugin-id>". > > Perhaps, there is something in your testing environment that affects the > Build ID label. Thanks Kaloyan. Stefan, do you also have a solution for the label? Identified the issue, I'm working on a fix. All should be good now with http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=e2d1173dd9e18faedde9d352a5aa9243fddad444 Thanks Stefan and Kaloyan. I think a downport is not an option, we typically only download critical features and this one depends on the few inherit support from Stefan. Lars, thanks for bringing this to a complete solution! Indeed, it's not a critical issue, so it's fair enough to be in Mars only. Verified in 4.5.0.N20150124-1500 |