Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 363053 - [CSS] CSS engine ignores alternative element providers
Summary: [CSS] CSS engine ignores alternative element providers
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.2   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Platform-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-07 10:20 EST by Brian de Alwis CLA
Modified: 2012-01-24 13:27 EST (History)
2 users (show)

See Also:

Attachments
demo app with custom element provider for Link (97.98 KB, application/zip)
2011-11-10 11:18 EST, Brian de Alwis CLA 		no flags Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Brian de Alwis CLA 2011-11-07 10:20:04 EST 
I started to investigate what would be necessary to hook in a custom element provider.  Although I can see my custom provider is picked up by CSSEngineImpl, it's never actually used.  Tracing through AbstractCSSEngine#getElement(Object) lines 844 - 849 seems incorrect: 

		if (element instanceof Element)
			elt = (Element) element;
		else if (elementProvider != null) {
			elt = elementProvider.getElement(element, this);
		} else if (elementProvider == null) {
			Object tmp = widgetsMap.get(element.getClass().getName());
			if (tmp == null) {
				Class parent = element.getClass();
				do {
					parent = parent.getSuperclass();
					tmp = widgetsMap.get(parent.getName());
				} while (tmp == null && parent != Object.class);
			}
			//found && parentClass != ElementAdapter.class     // line 844
			elementProvider = (IElementProvider) tmp;               // PROBLEM
			// Looks like this can be NULL see bug 338756
			if( elementProvider != null ) {
				elt = elementProvider.getElement(element, this);	
			}
		}

This code causes the first matching element provider found to become *the* only element provider consulted.

I think it should instead be:

			if(tmp != null && tmp instanceof IElementProvider) {
				elt = ((IElementProvider)tmp).getElement(element, this);
			}

This has the expected effect.
Comment 1 Brian de Alwis CLA 2011-11-07 11:50:07 EST 
This fix will have a performance impact when we enable dynamic pseudo classes (bug 362532).
Comment 2 Bogdan Gheorghe CLA 2011-11-09 15:02:16 EST 
Brian, did you add an extension point for your element provider? (org.eclipse.e4.css.core.elementProvider - take a look at the plugin.xml for org.eclipse.e4.ui.css.swt)
Comment 3 Brian de Alwis CLA 2011-11-10 11:18:30 EST 
Created attachment 206794 [details]
demo app with custom element provider for Link

(In reply to comment #2)
> Brian, did you add an extension point for your element provider?
> (org.eclipse.e4.css.core.elementProvider - take a look at the plugin.xml for
> org.eclipse.e4.ui.css.swt)

Yes, I did.  With the fix I describe above, my custom element provider is correctly instantiated and picked up.

I've attached the test app here.  It is the RCP Mail example with an attempt to add an element for Links, ostensibly to add support for a new property "link-color" (which depends on bug 130444).

If you look at the code above for AbstractCSSEngine#getElement(Object), which is called for every widget during the traversal, you'll see that if elementProvider == null, it causes elementProvider to be set to 'tmp'.  Subsequent calls will always use that elementProvider.  The first call will usually be for a Shell, which is mapped to SWTElementProvider, and thus the SWTElementProvider will be used for all subsequent widgets, regardless of whether they provide a different provider.  It just happens to work as we only provide a single element provider anyways, SWTElementProvider.
Comment 5 Brian de Alwis CLA 2012-01-24 13:27:56 EST 
Verified with demo app in I20120123-2200