Community
Participate
Working Groups
There are many cases where we use Widget.setData(keyString, value) to remember some CSS value. See SWTElement for many examples. These key strings are often of the form "active", "hover", etc. They should be fully qualified to reduce conflicts since others may be using the widget's setData. So should instead be: "active" -> "org.eclipse.e4.ui.css.swt.active" "hover" -> "org.eclipse.e4.ui.css.swt.hover" Also, we should have final static fields to hold keys with these as values, eg. in DynamicPseudoClassesSWTActiveHandler we would have final static String WIDGET_ACTIVE_KEY = "org.eclipse.e4.ui.css.swt.activeLost"
Created attachment 128415 [details] .setData() now uses qualified names
You have copy/paste errors: + public static final String FOCUS_LOST = "org.eclipse.e4.ui.css.swt.selectors.FOCUS_KEY"; + public static final String HOVER_LOST = "org.eclipse.e4.ui.css.swt.selectors.FOCUS_KEY"; + public static final String HOVER_LISTENER = "org.eclipse.e4.ui.css.swt.selectors.FOCUS_KEY"; Identical key value repeated for all three keys. Also this change looked odd: - return control.getData("mouseHover") != null; + return control.getData(CSSSWTConstants.HOVER_LOST) != null; Shouldn't the constant have been called "MOUSE_HOVER" to match the existing value? ie. did you intend on renaming it or is this an error?
Also note inconsistent naming of vars vs. values e.g. + public static final String FOCUS_LOST = "org.eclipse.e4.ui.css.swt.selectors.FOCUS_KEY"; This one used the word "key" in the value but not in the key, which seemed odd, but others didn't.
Note that qualified key value strings should generally follow the package structure. Specifically, you have: org.eclipse.e4.ui.css.swt.selectors.XXX but that's not the package for CSSSWTConstants (ie. 'selectors').
Please correct all these items and resubmit the patch.
Created attachment 131290 [details] qualified updated
I noticed the key names and values don't match for these: public static final String MOUSE_HOVER = "org.eclipse.e4.ui.css.swt.HOVER_LOST"; public static final String MOUSE_HOVER_LOST = "org.eclipse.e4.ui.css.swt.HOVER_LISTENER"; Is this intentional, and if so why?
Released to HEAD
