Bugzilla – Full Text Bug Listing
| Summary: | [CSS] css background image does not work in rcp apps | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] e4 | Reporter: | Kai Toedter <kai.toedter> | ||||
| Component: | UI | Assignee: | Kevin McGuire <Kevin_McGuire> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | azerr, Kevin_McGuire, Matthew_Hatem, pwebster | ||||
| Version: | 0.9 | ||||||
| Target Milestone: | 0.9 M4 | ||||||
| Hardware: | PC | ||||||
| OS: | Windows Vista | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Kai Toedter
(I assume by "css demo" you're referring to the CSS editor example where you can type in the style sheet). Yes, there's work that needs to be done here. The css editor example is a Java app. It just does: FileInputStream(new File(uri)); in FileResourcesLocatorImpl. By contrast, the path resolution for RCP apps should be based on the bundle location, in the same way we're looking up the style sheet in the photo demo. The good news is that the resource lookup/resolution is pluggable. What we need is to register a different IResourceLocator for java app vs. OSGi based app. (In reply to comment #1) > (I assume by "css demo" you're referring to the CSS editor example where you > can type in the style sheet). > > Yes, there's work that needs to be done here. The css editor example is a Java > app. It just does: > > FileInputStream(new File(uri)); > > in FileResourcesLocatorImpl. > > By contrast, the path resolution for RCP apps should be based on the bundle > location, in the same way we're looking up the style sheet in the photo demo. > > The good news is that the resource lookup/resolution is pluggable. What we need > is to register a different IResourceLocator for java app vs. OSGi based app. > Yes kevin you have right. At first we must implement for OSGI an IResourceLocator (ex : BundleResourceLocatorImpl (or PluginResourceLocatorImpl or OSGIResourceLocatorImpl)). After we must register the BundleResourceLocatorImpl into the IResourcesLocatorManager of CSS engine. Today AbstractCSSEngien use an instance of ResourcesLocatorManager(which implements IResourcesLocatorManager) which register FileResourcesLocatorImpl (to manage local file) and HttpResourcesLocatorImpl (to manage http file) into the constructor. Perhaps it was a bad idea to register locator into the constrctor? So we must create new manager BundleResourcesLocatorManager to manage OSGI which extend ResourcesLocatorManager and register BundleResourceLocatorImpl. BundleResourcesLocatorManager can be a singleton. To use it into CSS engien we must call CSSEngine#setResourcesLocatorManager(IResourcesLocatorManager manager). Perhaps it shoud be cool to manage several context like platform://, plugin:// (or bundle://) and so have 2 *ResourceLocatorImpl. Hope my explanation will help you. Regards Angelo (In reply to comment #2) > Perhaps it was a bad idea to register locator into the constrctor? So we must > create new manager BundleResourcesLocatorManager to manage OSGI which extend > ResourcesLocatorManager and register BundleResourceLocatorImpl. > BundleResourcesLocatorManager can be a singleton. Yes this is the typical problem with pure Java - when you don't have extension points, you end up having to do things like this in constructors and it's brittle. The typical eclipse solution pattern is to add an extension point for resource locators and that's how they get into the manager. Different environments add different kinds. But, Angelo I think in the past you thought it important for us to support the non OSGi world. This is quite restrictive for us though, and the topic has come up previously around the need for a hierarchy of engines (v.s. a single one with capabilities added via extension). Note presently all the CSS projects, including the examples, are plugin projects because that makes it much easier to manage in Eclipse. From what I've seen, I believe that almost always people will be using the e4 CSS support in plugin projects in an RCP app, not in pure SWT apps. I didn't really want to hijack this bug for this OSGi strategic discussion, but I believe it's at the route of what technical solution we pick here. (In reply to comment #3) Just to make clear what I'm suggesting: We should assume we run in an OSGi world, add extension points for extensible aspects of the design, and in this particular case, change the resource lookup to be bundle relative. We would forgo the ability to run non-OSGi. Of course, if hordes of people showed up saying they wanted to run CSS without OSGi (and wanted to contribute) I'd reconsider, but given limited resources to apply to this problem, I'd rather go the OSGi enabled route, which I believe is the most common real usage. (In reply to comment #4) > (In reply to comment #3) > > Just to make clear what I'm suggesting: > We should assume we run in an OSGi world, add extension points for extensible > aspects of the design, and in this particular case, change the resource lookup > to be bundle relative. We would forgo the ability to run non-OSGi. > > Of course, if hordes of people showed up saying they wanted to run CSS without > OSGi (and wanted to contribute) I'd reconsider, but given limited resources to > apply to this problem, I'd rather go the OSGi enabled route, which I believe is > the most common real usage. > Kevin, in my old company they are develeloping a big application without OSGI context and they would like perhaps integrate CSS engine. I know that that someone have integrate CSS engine with FormToolkit and SWT Designer. I'm not sure that there is OSGI context in this case? This choice is very important. Me I will prefer provide several IResourcesLocator implementation to manage OSGI and no OSGI. (In reply to comment #5) > Kevin, in my old company they are develeloping a big application without OSGI > context and they would like perhaps integrate CSS engine. I know that that > someone have integrate CSS engine with FormToolkit and SWT Designer. I'm not > sure that there is OSGI context in this case? This choice is very important. Me > I will prefer provide several IResourcesLocator implementation to manage OSGI > and no OSGI. Yes but it's not just IResourcesLocator (if it was, I'd be inclined to just split the projects for OSGi v.s. non), it's also things like the hierarchy of engines which creates a real problem if you want to add new widgets. We shouldn't have to have a nebula engine, we should just add via extension the CSS support for nebula widgets. As soon as you add more widgets you then are stuck because you can only have a single parented hierarchy of engines where as you actually have a mixin of widget types. (In reply to comment #6) > (In reply to comment #5) > > > Kevin, in my old company they are develeloping a big application without OSGI > > context and they would like perhaps integrate CSS engine. I know that that > > someone have integrate CSS engine with FormToolkit and SWT Designer. I'm not > > sure that there is OSGI context in this case? This choice is very important. Me > > I will prefer provide several IResourcesLocator implementation to manage OSGI > > and no OSGI. > > Yes but it's not just IResourcesLocator (if it was, I'd be inclined to just > split the projects for OSGi v.s. non), it's also things like the hierarchy of > engines which creates a real problem if you want to add new widgets. We > shouldn't have to have a nebula engine, we should just add via extension the > CSS support for nebula widgets. As soon as you add more widgets you then are > stuck because you can only have a single parented hierarchy of engines where as > you actually have a mixin of widget types. > Ok I understand the problem. The idea is to have a SWTCSSEngienImpl and add support for any widget. Problem comes from that this initialisation is done into constructor. So why delegate this initialisation to another classes (CSSEngineConfiguration?) For no OSGI CSSEngineConfiguration is done like today, for OSGI we use extension point. I don't know if it works, it's just an idea. (In reply to comment #7) > Ok I understand the problem. The idea is to have a SWTCSSEngienImpl and add > support for any widget. Problem comes from that this initialisation is done > into constructor. Yes that's right. You could imagine in conjunction with the "element class per widget class" pattern (either singleton or not, as discussed elsewhere) that for each widget an element class would be added via extension. > So why delegate this initialisation to another classes > (CSSEngineConfiguration?) For no OSGI CSSEngineConfiguration is done like > today, for OSGI we use extension point. I don't know if it works, it's just an > idea. The problem is bootstrapping. Imagine you introduce a new widget to the IDE (say in some view), and you provide the CSS element class for the styling. The mapping is expressed in a CSSEngineConfiguration. In the OSGi case, how do you know to add it to the engine? The engine is not going to tell you, "Everyone, please add your configs now" because it would need to do so via an extension point and in this scenario it's non OSGi. Your only choice is to make your plugin early startup, which causes it to be initialized and run on every Eclipse start, so that you have a chance to add your config. This is a bad practice. So, you need something like an OSGi enabled engine, since the engine is at the bottom of everything, and another non-OSGi engine which just has lists that get added to on startup. It's non trivial to set up and maintain both (not to mention the testing). I guess where you'd a hierarchy like: CSSEngine (abstract) SWTEngine OSGiEngine NonOSGiEngine But I'm going to want to add all the Nebula widgets CSS support via extension point. I don't know how you get to reuse that for the NonOSGiEngine (unless you again have two versions of how you add the Nebula support). Another solution for OSGi could be to use the Extender Pattern and provide a new custom manifest header just for css styling. Extenders are commonly used, e.g. OSGi Declarative Services and Spring DM use Extenders. (In reply to comment #9) > Another solution for OSGi could be to use the Extender Pattern and provide a > new custom manifest header just for css styling. Extenders are commonly used, > e.g. OSGi Declarative Services and Spring DM use Extenders. Your OSGi skills are superior to mine :) I don't know how that works. @Kevin The first question is: Shall we allow css styling on a bundle basis rather than an application basis? If so, how could we merge bundle based styling with application based styling? But if we think about styling of bundles that might dynamically come and go, the extender patter might help. Here is a brief description how it works: 1) We provide a new OSGi manifest header, e.g. x-css-path: css/bundle.css 2) We implement a bundle tracker that tracks the dynamic coming and going of bundles. This bundle tracker might use a kind of CSS Engine servive to get the current engine. 3) Every time a new bundle comes in that provides the above header, we pass over the style information to the engine and apply it to all UI contributions the bundle provides (no idea how to do that right now) The advantage using this pattern rather than an extension point is that extension are bound to the "resolved" state of a bundle, while this solution is bound to the "active" state of a bundle (so it is really dynamic) (In reply to comment #11) > The first question is: Shall we allow css styling on a bundle basis rather than > an application basis? If so, how could we merge bundle based styling with > application based styling? I *think* it's the application level but I may be misinterpreting what you mean. I don't know if this addresses your question, but how I'm guessing it'll work is: 1) The IDE framework defines the structure of the UI, specifying possible CSS classnames and IDs. 2) The product defines the base style sheet (can be replaced). 3) As new widgets are brought in via extension, so too are default styles for them against (2). 4) As new views and editors are brought in, they will as much as possible use (1). They may also define their own (e.g. Java editor defines many new color constants). > But if we think about styling of bundles that might dynamically come and go, > the extender patter might help. Here is a brief description how it works: Thanks for the description. For the short term, I want to do the simplest thing that gets you going. Maybe the solution is in the modelled UI application and the code by which it configures the CSS engine. We could write the bundle based resource lookup in a separate project and add it to the engine on create. Maybe we don't add the file system based one by default as a result. There's no automatic configuration of the engine via extension point (for now anyway). I've taken ownership of this bug because I marked it for M4 (so somebody needs to do the work <g>). However, others should feel free to contribute or take the bug, in which case let me know if you're starting on that so we don't duplicate effort. I have a preliminary implementation of this. The general shape is as follows: 1) Introduced class OSGiResourceLocator 2) The ResourcesLocatorManager no longer by defaults adds the FileResourcesLocatorImpl. This is required because at present the first locator that resolves the URI string wins, and FileResourcesLocatorImpl always resolves. 3) The non-OSGi examples now must add it FileResourcesLocatorImpl explicitly. I've fixed the SWT example but not the nebula yet. 4) OSGiResourceLocator works by being given a base string. It appends the file name to the end then does a core FileLocator.resolve(). 5) Modelled UI based apps can now add a property which will be that base string. 6) If the property is absent for Modelled UI based apps, the OSGiResourceLocator isn't created. I'm not sure though that the approach of appending the file name onto the path is the best approach. Created attachment 138735 [details]
Patch with initial implementation of OSGi based resource lookup
(In reply to comment #14) > I'm not sure though that the approach of appending the file name onto the path > is the best approach. Meant to add, because: 1) it gives only a single location for all images 2) appends the file name to the end, which maybe isn't cool 3) requires the path have a '/' at end (but maybe we could add one for free if missing). Re: comment #16 > 3) requires the path have a '/' at end (but maybe we could add one for free if missing). We should support paths with and without '/' I like the overall approach. I guess I don't see why we cannot support platform URLs directly. I assume it's because of the OSGi/non-OSGi portability concerns. In our implementation we supported platform URLs as well as relative URLs. In the case of a relative URL the image locations were resolved relative to the location of the CSS file. (In reply to comment #17) > I like the overall approach. I guess I don't see why we cannot support > platform URLs directly. I assume it's because of the OSGi/non-OSGi portability > concerns. What do you mean by "directly"? Maybe we can, how would it work? > In our implementation we supported platform URLs as well as relative URLs. In > the case of a relative URL the image locations were resolved relative to the > location of the CSS file. I like this idea. I had thought of something somewhat related like: background-image: url(bundle://images/prettypic.gif) where 'bundle://' means "the bundle where the CSS was defined", under the assumption that people almost always would define the CSS and resources in the same bundle. I've released the patch as is since I wanted Kai to have something to work with asap. I'll open a new bug to track some the future looking ideas. If there are problems with the released code, either enter new bugs or reopen this one. (In reply to comment #19) > I'll open a new bug to track some the future looking ideas. See bug #279838 |