Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 363736

Summary:	Default theme modifications severely affect consumer themes
Product:	[RT] RAP	Reporter:	Austin Riddle <austin.riddle>
Component:	RWT	Assignee:	Project Inbox <rap-inbox>
Status:	RESOLVED FIXED	QA Contact:
Severity:	normal
Priority:	P3	CC:	ivan, mseashore, ronald.so, tbuschto
Version:	1.5
Target Milestone:	1.5 M5
Hardware:	All
OS:	Windows 7
Whiteboard:

Description Austin Riddle

2011-11-14 14:03:47 EST

As we have had lately some drastic theme changes, here are some points/issues we should consider:

Consumer Themes:
1) Consumer themes are defined in a single file, which makes comparing to individual widget default.css files difficult.
2) Consumer themes that only override a subset of the default styles become sensitive to default theme changes. Even if the consumers override all the default styles, additions to the default theme will still disturb them (For instance, when new theme functionality is added that was not available previously).
3) Consumer themes do not have the ability to *replace* the default theme, only override.

Default Themes:
1) We want RAP to have an attractive theme that works "out-of-the-box". This however is complicated because what is attractive really depends on what is trendy at the time, which implies that it will change.
2) The default theme cannot just be a particular platform's SWT theme. This would require us to maintain multiple themes.


Due to recent changes, all of our themes have had to be reworked with some significant effort. To avoid having to go through that again, the solution that I propose to address all of these issues is to allow a consumer to specify that their theme *replaces* the default theme. That way, any changes tot he default theme will not affect consumer themes.

Does anyone have any other ideas as to how we can prevent these kind of undesirable situations?

Comment 1 Ralf Sternberg

2011-11-23 08:56:05 EST

In fact, a custom theme *does* replace the default theme. However, for properties missing in the custom theme, the default theme is still used as a fallback. I understand that this silent fallback causes the problem described here. It's hard to identify exactly which properties are taken from the default theme.

The theming _needs_ some value for every property, therefore I think there has to be some kind of fallback. But maybe the issue here is that our default theme does much more than providing a fallback.

Here's my suggestion:
1) We provide a built-in fallback theme without any graphical effects. It only provides meaningful defaults for all properties. With this change, it's less likely that a fallback screws up a custom theme.

2) We generate some kind of warning in development mode if a custom theme misses some properties. This makes it easier to identify holes in a custom theme.

What do you think?

Comment 2 Tim Buschtoens

2011-11-23 10:06:28 EST

I think having "neutral" fallback values is the way to go. Not sure we still need the warning if we have that.

Comment 3 Ivan Furnadjiev

2011-11-23 10:11:56 EST

We could use this "neutral" fallback for execution of server-side tests as well.

Comment 4 Tim Buschtoens

2011-11-23 10:17:25 EST

(In reply to comment #3)
> We could use this "neutral" fallback for execution of server-side tests as
> well.

Client-side also!

Comment 5 Austin Riddle

2011-11-28 10:52:58 EST

(In reply to comment #1)

> Here's my suggestion:
> 1) We provide a built-in fallback theme without any graphical effects. It only
> provides meaningful defaults for all properties. With this change, it's less
> likely that a fallback screws up a custom theme.
> 
> 2) We generate some kind of warning in development mode if a custom theme
> misses some properties. This makes it easier to identify holes in a custom
> theme.
> 

I also like the idea of a more neutral fallback. And I do think we need some simple way to determine if your custom theme is not completely overriding the default fallback. There are 2 use cases: partial and complete override of the default theme. The complication here is that in the partial case you don't want warnings showing up. In the complete case you do indeed want to know.
Could we support both?  Or should we just say that custom themes always should override the default or you will get a warning?

Comment 6 Ralf Sternberg

2011-11-28 12:08:19 EST

I implemented warnings for incomplete themes in bug 254478. For now, these warnings have to be enabled by setting the system property "org.eclipse.rap.enableThemeWarnings" to "true". They will only be issued in development (debug) mode.

Comment 7 Austin Riddle

2011-11-28 16:18:22 EST

(In reply to comment #6)
> I implemented warnings for incomplete themes in bug 254478. For now, these
> warnings have to be enabled by setting the system property
> "org.eclipse.rap.enableThemeWarnings" to "true". They will only be issued in
> development (debug) mode.

Looks good to me.

Comment 8 Ivan Furnadjiev

2012-01-11 08:07:09 EST

Introduced fall-back theme - based on *.default.css (we decided to keep the name of the files). The default theme is now located in "org.eclipse.rap.rwt\resources\resource\theme\default.css". Client and server tests are executed against fall-back theme. Next step is to reduce the fall-back theme - remove unnecessary definitions.

Comment 9 Ivan Furnadjiev

2012-01-11 14:26:37 EST

Reduced the fall-back theme. With this change default theme modifications do not affect consumer themes anymore. Changes are in CVS.