This Bugzilla instance is deprecated, and most Eclipse projects now use GitHub or Eclipse GitLab. Please see the deprecation plan for details.
Bug 420836 - [CSS] Migrate CSS themes from eclipse.platform repo to eclipse.platform.ui repo
Summary: [CSS] Migrate CSS themes from eclipse.platform repo to eclipse.platform.ui repo
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.4   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 4.4 M6   Edit
Assignee: Lars Vogel CLA
QA Contact: Lars Vogel CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-10-31 17:07 EDT by Lars Vogel CLA
Modified: 2014-03-04 04:20 EST (History)
7 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Lars Vogel CLA 2013-10-31 17:07:48 EDT
I think we should move the CSS themes from org.eclipse.platform to a new plug-in. It makes more sense IMHO to maintain as part of the UI team.

I happy to move the CSS themes to a new plug-in. Any disagreement?
Comment 1 Lars Vogel CLA 2013-11-03 04:44:27 EST
The other advantage I see by moving them into their own plug-in is that RCP applications could directly consume the framework styling.
Comment 2 Paul Webster CLA 2013-11-14 12:42:35 EST
Would you move them to their own plugin and then  contribute bug 416218 into it?

If the platform is providing it, it doesn't seem like we need one plugin for dark and one plugin for 2 other themes.

PW
Comment 3 Lars Vogel CLA 2013-11-14 13:05:16 EST
(In reply to Paul Webster from comment #2)
> Would you move them to their own plugin and then  contribute bug 416218 into
> it?
> 
> If the platform is providing it, it doesn't seem like we need one plugin for
> dark and one plugin for 2 other themes.

OK. I suggest "org.eclipse.ui.themes" as plug-in name. Let me know if you disagree otherwise, I start the migration.
Comment 4 Paul Webster CLA 2013-11-14 13:15:36 EST
Would one of our existing low-level plugins be an appropriate place to put the Platform themes?

ex: org.eclipse.e4.ui.css.swt.theme or org.eclipse.e4.ui.workbench.swt ?

It looks like a theme is some CSS,images, and entries in a plugin.xml.

PW
Comment 5 Lars Vogel CLA 2013-11-14 13:46:03 EST
(In reply to Paul Webster from comment #4)
> Would one of our existing low-level plugins be an appropriate place to put
> the Platform themes?
> 
> ex: org.eclipse.e4.ui.css.swt.theme or org.eclipse.e4.ui.workbench.swt ?
> 
> It looks like a theme is some CSS,images, and entries in a plugin.xml.

I think we should separate the implementation from the examples. Assume an RCP client wants to use theming support with his custom theme and adds the implementation plug-ins to his RCP product. If we re-use the existing plug-ins we would also get the platform themes which he potentially does not want.
Comment 6 John Arthorne CLA 2013-11-14 13:52:45 EST
(In reply to Lars Vogel from comment #5)
> I think we should separate the implementation from the examples. Assume an
> RCP client wants to use theming support with his custom theme and adds the
> implementation plug-ins to his RCP product. If we re-use the existing
> plug-ins we would also get the platform themes which he potentially does not
> want.

The built in themes would automatically be available, but the RCP app would not be forced to use or expose them right? I understand it is not a perfect fit, but creating a new plugin for a handful of CSS files feels like overkill. If there is somewhere else we can store them that does not do any harm to those who don't want to use them, I prefer to avoid the overhead.
Comment 7 Lars Vogel CLA 2013-11-14 13:57:42 EST
> The built in themes would automatically be available, but the RCP app would
> not be forced to use or expose them right? I understand it is not a perfect
> fit, but creating a new plugin for a handful of CSS files feels like
> overkill. If there is somewhere else we can store them that does not do any
> harm to those who don't want to use them, I prefer to avoid the overhead.

I think adding the CSS files to an implementation plug-in would create potential for others. 
 
Take our Contacts demo for example. It uses a model processor to read all installed themes and creates dynamically user interface components for them. 

Or lets assume someone want to provide a different themed Eclipse, e.g. Liclipse for PHP or SAP with their Eclipse based tooling  Adding the themes to the implementation plug-in would potentially "polute" their IDEs.
Comment 8 Lars Vogel CLA 2013-11-14 13:58:12 EST
s/potential/potential work
Comment 9 Lars Vogel CLA 2013-11-14 15:03:24 EST
We discussed that in the eclipse-e4 IRC channel and to avoid that customers or custom IDE gets these themes unwanted we agreed to create a new plug-in called "org.eclipse.ui.e4.themes" to hold the existing themes from eclipse platform and the new dark theme.
Comment 10 Lars Vogel CLA 2014-02-19 06:59:46 EST
Removal from the eclipse.platform repo

https://git.eclipse.org/r/22228
Comment 11 Lars Vogel CLA 2014-02-19 07:01:38 EST
Adding the themes to org.eclipse.ui.themes in to eclipse.platform.ui

https://git.eclipse.org/r/22229
Comment 13 Lars Vogel CLA 2014-02-20 11:09:36 EST
Fixes with the two commmits from Paul.
Comment 14 Phil Beauvoir CLA 2014-02-21 04:29:54 EST
I included org.eclipse.ui.themes in my RCP application using Build id: I20140218-0800.

When I tried to switch to the dark theme I got an error:

java.net.MalformedURLException
	at java.net.URL.<init>(URL.java:619)
	at java.net.URL.<init>(URL.java:482)
	at java.net.URL.<init>(URL.java:431)
	at org.eclipse.e4.ui.css.core.impl.engine.AbstractCSSEngine.parseStyleSheet(AbstractCSSEngine.java:203)
	at org.eclipse.e4.ui.css.swt.internal.theme.ThemeEngine.setTheme(ThemeEngine.java:421)
	at org.eclipse.e4.ui.css.swt.internal.theme.ThemeEngine.setTheme(ThemeEngine.java:382)
	at org.eclipse.e4.ui.css.swt.internal.theme.ThemeEngine.restore(ThemeEngine.java:546)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.initializeStyling(PartRenderingEngine.java:1248)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$9.run(PartRenderingEngine.java:1009)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1006)
	at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:147)
	at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:620)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:571)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:150)
	at com.archimatetool.editor.Application.start(Application.java:73)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196)
Comment 15 Lars Vogel CLA 2014-02-21 04:34:53 EST
(In reply to Phillipus B from comment #14)
> I included org.eclipse.ui.themes in my RCP application using Build id:
> I20140218-0800.

Can you please open a new bug (Use platform themes in RCP) and copy me into the bug report. I have a look.
Comment 16 Phil Beauvoir CLA 2014-02-21 04:43:06 EST
(In reply to Lars Vogel from comment #15)
> (In reply to Phillipus B from comment #14)
> > I included org.eclipse.ui.themes in my RCP application using Build id:
> > I20140218-0800.
> 
> Can you please open a new bug (Use platform themes in RCP) and copy me into
> the bug report. I have a look.

Bug #428715
Comment 17 Dani Megert CLA 2014-02-21 08:36:29 EST
Are you guys 100% sure that clients aren't referring / importing our css files, e.g. like this:

@import url("platform:/plugin/org.eclipse.ui.themes/css/e4_basestyle.css");

?
Comment 18 Lars Vogel CLA 2014-02-21 08:44:47 EST
(In reply to Dani Megert from comment #17)
> Are you guys 100% sure that clients aren't referring / importing our css
> files, e.g. like this:
> 
> @import url("platform:/plugin/org.eclipse.ui.themes/css/e4_basestyle.css");
> 

AFAIK CSS imports did not work before Luna. I started to support it with https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=92ed79170250f3b070e129d54c165b31a9a430dd and the last fixes make platform:/plugin/ finally work.
Comment 19 Lars Vogel CLA 2014-02-21 08:47:02 EST
(In reply to Lars Vogel from comment #18)
> (In reply to Dani Megert from comment #17)
> > Are you guys 100% sure that clients aren't referring / importing our css
> > files, e.g. like this:
> > 
> > @import url("platform:/plugin/org.eclipse.ui.themes/css/e4_basestyle.css");
> > 
> 
> AFAIK CSS imports did not work before Luna. I started to support it with
> https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/
> ?id=92ed79170250f3b070e129d54c165b31a9a430dd and the last fixes make
> platform:/plugin/ finally work.

The only way to import a CSS before Luna was to use "/css/e4_basestyle.css" because we hard-code the bundle symbolic name. If you want we can easily turn this again on. I actually just removed it. We would simply point to org.eclipse.ui.themes instead of org.eclipse.platform. "platform:/plugin/" did not work before my changes so we should be save.
Comment 20 Paul Webster CLA 2014-02-21 08:48:32 EST
We still have reference over the web:

https://wiki.eclipse.org/Eclipse4/CSS
http://waynebeaton.wordpress.com/2012/02/28/editing-the-eclipse-4-workbench-css/
http://stackoverflow.com/questions/20455110/how-to-import-a-resource-in-eclipse-plugin-stylesheet

They probably need a comment.

eclipse.platform/platform/org.eclipse.platform/plugin.xml still refers to the org.eclipse.platform as a place to get CSS resource information(images), so org.eclipse.sdk probably does as well.

The images should go with o.e.ui.themes and that should be our default location, but we'll need to leave the existing ones there with a note that they're not used by the SDK any more.

o.e.ui.themes should still  be the plugin that defines the themes in the plugin.xml

PW
Comment 21 Paul Webster CLA 2014-02-21 08:48:48 EST
.
Comment 22 Lars Vogel CLA 2014-02-21 08:58:30 EST
(In reply to Paul Webster from comment #21)
> .

Gerrit review for the adjustment of the SDK resource location
https://git.eclipse.org/r/#/c/22368
Comment 23 Dani Megert CLA 2014-02-21 09:00:45 EST
And the 'build.properties' file also needs love i.e. remove the entries for removed stuff.
Comment 24 Lars Vogel CLA 2014-02-21 09:05:17 EST
(In reply to Dani Megert from comment #17)
> Are you guys 100% sure that clients aren't referring / importing our css
> files, e.g. like this:
> 
> @import url("platform:/plugin/org.eclipse.ui.themes/css/e4_basestyle.css");
> 

The old import scenario should work again, as I placed the default URL again in

https://git.eclipse.org/r/#/c/22353/
Comment 25 Lars Vogel CLA 2014-02-21 09:07:20 EST
(In reply to Dani Megert from comment #23)
> And the 'build.properties' file also needs love i.e. remove the entries for
> removed stuff.

Included in https://git.eclipse.org/r/#/c/22368/
Comment 26 Lars Vogel CLA 2014-02-21 09:11:01 EST
I opened Bug 428744 for EPP
Comment 27 Paul Webster CLA 2014-02-24 09:30:09 EST
(In reply to Lars Vogel from comment #24)
> 
> The old import scenario should work again, as I placed the default URL again
> in
> 
> https://git.eclipse.org/r/#/c/22353/

Released as http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=1b58a7af4ebb93f4d66f2a6466d155d5fc31c60f

(In reply to Lars Vogel from comment #25)
> 
> Included in https://git.eclipse.org/r/#/c/22368/

Released as http://git.eclipse.org/c/platform/eclipse.platform.git/commit/?id=9cfbe9deb857aa2f0f1fc0b88c1b2b44c5fc167a

PW
Comment 28 Lars Vogel CLA 2014-02-26 04:23:31 EST
Fixed with the commits from Paul
Comment 29 Lars Vogel CLA 2014-03-04 04:20:01 EST
All themes work fine in Build id: I20140303-2000