bug 304285 - Florian Thienel's patch "Cleanup of StyleSheet" reviewed and applied
diff --git a/sourceediting/plugins/org.eclipse.wst.xml.vex.core/src/org/eclipse/wst/xml/vex/core/internal/css/StyleSheet.java b/sourceediting/plugins/org.eclipse.wst.xml.vex.core/src/org/eclipse/wst/xml/vex/core/internal/css/StyleSheet.java
index 7c82dcf..595dec0 100644
--- a/sourceediting/plugins/org.eclipse.wst.xml.vex.core/src/org/eclipse/wst/xml/vex/core/internal/css/StyleSheet.java
+++ b/sourceediting/plugins/org.eclipse.wst.xml.vex.core/src/org/eclipse/wst/xml/vex/core/internal/css/StyleSheet.java
@@ -17,10 +17,10 @@
import java.io.Serializable;
import java.lang.ref.WeakReference;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
-import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.WeakHashMap;
@@ -97,14 +97,9 @@
IProperty.AXIS_VERTICAL), new BorderSpacingProperty(), };
/**
- * The properties to calculate. This can be changed by the app.
- */
- private static IProperty[] properties = CSS_PROPERTIES;
-
- /**
* The rules that comprise the stylesheet.
*/
- private Rule[] rules;
+ private final List<Rule> rules;
/**
* Computing styles can be expensive, e.g. we have to calculate the styles
@@ -124,8 +119,8 @@
* @param rules
* Rules that constitute the style sheet.
*/
- public StyleSheet(Rule[] rules) {
- this.rules = rules;
+ public StyleSheet(final Collection<Rule> rules) {
+ this.rules = new ArrayList<Rule>(rules);
}
/**
@@ -173,13 +168,6 @@
}
/**
- * Returns the array of standard CSS properties.
- */
- public static IProperty[] getCssProperties() {
- return CSS_PROPERTIES;
- }
-
- /**
* Returns the styles for the given element. The styles are cached to ensure
* reasonable performance.
*
@@ -195,7 +183,7 @@
// can't combine these tests, since calling ref.get() twice
// (once to query for null, once to get the value) would
// cause a race condition should the GC happen btn the two.
- styles = (Styles) ref.get();
+ styles = ref.get();
if (styles != null) {
return styles;
}
@@ -225,14 +213,14 @@
parentStyles = this.getStyles(element.getParent());
}
- Map<String, LexicalUnit> decls = getApplicableDecls(element);
+ Map<String, LexicalUnit> decls = getApplicableDeclarations(element);
LexicalUnit lu;
// If we're finding a pseudo-element, look at the 'content' property
// first, since most of the time it'll be empty and we'll return null.
if (element instanceof PseudoElement) {
- lu = (LexicalUnit) decls.get(CSS.CONTENT);
+ lu = decls.get(CSS.CONTENT);
if (lu == null) {
return null;
}
@@ -258,9 +246,8 @@
styles.setContent(content);
}
- for (int i = 0; i < properties.length; i++) {
- IProperty property = properties[i];
- lu = (LexicalUnit) decls.get(property.getName());
+ for (final IProperty property : CSS_PROPERTIES) {
+ lu = decls.get(property.getName());
Object value = property.calculate(lu, parentStyles, styles);
styles.put(property.getName(), value);
}
@@ -293,73 +280,49 @@
}
/**
- * Returns the list of properties to be parsed by StyleSheets in this app.
- */
- public static IProperty[] getProperties() {
- return StyleSheet.properties;
- }
-
- /**
* Returns the rules comprising this stylesheet.
*/
- public Rule[] getRules() {
- return this.rules;
+ public List<Rule> getRules() {
+ return Collections.unmodifiableList(this.rules);
}
-
- /**
- * Sets the list of properties to be used by StyleSheets in this
- * application.
- *
- * @param properties
- * New array of IProperty objects to be used.
- */
- public static void setProperties(IProperty[] properties) {
- StyleSheet.properties = properties;
- }
-
+
/**
* Returns all the declarations that apply to the given element.
*/
- private Map<String, LexicalUnit> getApplicableDecls(VEXElement element) {
- // Find all the property declarations that apply to this element.
- List<PropertyDecl> declList = new ArrayList<PropertyDecl>();
- Rule[] rules = this.getRules();
-
- for (int i = 0; i < rules.length; i++) {
- Rule rule = rules[i];
- if (rule.matches(element)) {
- PropertyDecl[] ruleDecls = rule.getPropertyDecls();
- for (int j = 0; j < ruleDecls.length; j++) {
- declList.add(ruleDecls[j]);
- }
- }
- }
+ private Map<String, LexicalUnit> getApplicableDeclarations(final VEXElement element) {
+ final List<PropertyDecl> rawDeclarationsForElement = findAllDeclarationsFor(element);
// Sort in cascade order. We can then just stuff them into a
// map and get the right values since higher-priority values
// come later and overwrite lower-priority ones.
- Collections.sort(declList, PROPERTY_CASCADE_ORDERING);
+ Collections.sort(rawDeclarationsForElement, PROPERTY_CASCADE_ORDERING);
- Map<String, PropertyDecl> decls = new HashMap<String, PropertyDecl>();
- Iterator<PropertyDecl> iter = declList.iterator();
- while (iter.hasNext()) {
- PropertyDecl decl = (PropertyDecl) iter.next();
- PropertyDecl prevDecl = (PropertyDecl) decls
- .get(decl.getProperty());
- if (prevDecl == null || !prevDecl.isImportant()
- || decl.isImportant()) {
- decls.put(decl.getProperty(), decl);
+ final Map<String, PropertyDecl> distilledDeclarations = new HashMap<String, PropertyDecl>();
+ final Map<String, LexicalUnit> values = new HashMap<String, LexicalUnit>();
+ for (final PropertyDecl declaration : rawDeclarationsForElement) {
+ final PropertyDecl previousDeclaration = distilledDeclarations.get(declaration.getProperty());
+ if (previousDeclaration == null || !previousDeclaration.isImportant()
+ || declaration.isImportant()) {
+ distilledDeclarations.put(declaration.getProperty(), declaration);
+ values.put(declaration.getProperty(), declaration.getValue());
}
}
- Map<String, LexicalUnit> values = new HashMap<String, LexicalUnit>();
- for (Iterator<String> it = decls.keySet().iterator(); it.hasNext();) {
- PropertyDecl decl = (PropertyDecl) decls.get(it.next());
- values.put(decl.getProperty(), decl.getValue());
- }
-
return values;
}
+
+ private List<PropertyDecl> findAllDeclarationsFor(final VEXElement element) {
+ final List<PropertyDecl> rawDeclarations = new ArrayList<PropertyDecl>();
+ for (final Rule rule : rules) {
+ if (rule.matches(element)) {
+ PropertyDecl[] ruleDecls = rule.getPropertyDecls();
+ for (final PropertyDecl ruleDecl : ruleDecls) {
+ rawDeclarations.add(ruleDecl);
+ }
+ }
+ }
+ return rawDeclarations;
+ }
private Map<VEXElement, WeakReference<Styles>> getStyleMap() {
if (styleMap == null) {
diff --git a/sourceediting/plugins/org.eclipse.wst.xml.vex.core/src/org/eclipse/wst/xml/vex/core/internal/css/StyleSheetReader.java b/sourceediting/plugins/org.eclipse.wst.xml.vex.core/src/org/eclipse/wst/xml/vex/core/internal/css/StyleSheetReader.java
index 6d76d13..66557a5 100644
--- a/sourceediting/plugins/org.eclipse.wst.xml.vex.core/src/org/eclipse/wst/xml/vex/core/internal/css/StyleSheetReader.java
+++ b/sourceediting/plugins/org.eclipse.wst.xml.vex.core/src/org/eclipse/wst/xml/vex/core/internal/css/StyleSheetReader.java
@@ -78,14 +78,12 @@
*/
public StyleSheet read(InputSource inputSource, URL url)
throws CSSException, IOException {
-
- Parser parser = createParser();
- List<Rule> rules = new ArrayList<Rule>();
- StyleSheetBuilder ssBuilder = new StyleSheetBuilder(rules, url);
- parser.setDocumentHandler(ssBuilder);
+ final Parser parser = createParser();
+ final List<Rule> rules = new ArrayList<Rule>();
+ final StyleSheetBuilder styleSheetBuilder = new StyleSheetBuilder(rules, url);
+ parser.setDocumentHandler(styleSheetBuilder);
parser.parseStyleSheet(inputSource);
- Rule[] ruleArray = rules.toArray(new Rule[rules.size()]);
- return new StyleSheet(ruleArray);
+ return new StyleSheet(rules);
}
// ======================================================== PRIVATE
diff --git a/sourceediting/tests/org.eclipse.wst.xml.vex.core.tests/src/org/eclipse/wst/xml/vex/core/internal/css/RuleTest.java b/sourceediting/tests/org.eclipse.wst.xml.vex.core.tests/src/org/eclipse/wst/xml/vex/core/internal/css/RuleTest.java
index d4a8338..64bc214 100644
--- a/sourceediting/tests/org.eclipse.wst.xml.vex.core.tests/src/org/eclipse/wst/xml/vex/core/internal/css/RuleTest.java
+++ b/sourceediting/tests/org.eclipse.wst.xml.vex.core.tests/src/org/eclipse/wst/xml/vex/core/internal/css/RuleTest.java
@@ -11,6 +11,7 @@
package org.eclipse.wst.xml.vex.core.internal.css;
import java.net.URL;
+import java.util.List;
import org.eclipse.wst.xml.vex.core.internal.css.Rule;
import org.eclipse.wst.xml.vex.core.internal.css.StyleSheet;
@@ -31,7 +32,7 @@
URL url = RuleTest.class.getResource("testRules.css");
StyleSheetReader reader = new StyleSheetReader();
StyleSheet ss = reader.read(url);
- Rule[] rules = ss.getRules();
+ List<Rule> rules = ss.getRules();
RootElement a = new RootElement("a");
Element b = new Element("b");
@@ -54,7 +55,7 @@
doc.insertElement(5, f);
// /* 0 */ c { }
- Rule rule = rules[0];
+ Rule rule = rules.get(0);
assertFalse(rule.matches(a));
assertFalse(rule.matches(b));
assertTrue(rule.matches(c));
@@ -62,7 +63,7 @@
assertFalse(rule.matches(e));
// /* 1 */ b c { }
- rule = rules[1];
+ rule = rules.get(1);
assertFalse(rule.matches(a));
assertFalse(rule.matches(b));
assertTrue(rule.matches(c));
@@ -70,7 +71,7 @@
assertFalse(rule.matches(e));
// /* 2 */ b d { }
- rule = rules[2];
+ rule = rules.get(2);
assertFalse(rule.matches(a));
assertFalse(rule.matches(b));
assertFalse(rule.matches(c));
@@ -78,7 +79,7 @@
assertFalse(rule.matches(e));
// /* 3 */ other b c { }
- rule = rules[3];
+ rule = rules.get(3);
assertFalse(rule.matches(a));
assertFalse(rule.matches(b));
assertFalse(rule.matches(c));
@@ -86,7 +87,7 @@
assertFalse(rule.matches(e));
// /* 4 */ other b d { }
- rule = rules[4];
+ rule = rules.get(4);
assertFalse(rule.matches(a));
assertFalse(rule.matches(b));
assertFalse(rule.matches(c));
@@ -94,7 +95,7 @@
assertFalse(rule.matches(e));
// /* 5 */ a c e { }
- rule = rules[5];
+ rule = rules.get(5);
assertFalse(rule.matches(a));
assertFalse(rule.matches(b));
assertFalse(rule.matches(c));
@@ -102,7 +103,7 @@
assertTrue(rule.matches(e));
// /* 6 */ c a e { }
- rule = rules[6];
+ rule = rules.get(6);
assertFalse(rule.matches(a));
assertFalse(rule.matches(b));
assertFalse(rule.matches(c));
@@ -110,7 +111,7 @@
assertFalse(rule.matches(e));
// /* 7 */ * { }
- rule = rules[7];
+ rule = rules.get(7);
assertTrue(rule.matches(a));
assertTrue(rule.matches(b));
assertTrue(rule.matches(c));
@@ -118,7 +119,7 @@
assertTrue(rule.matches(e));
// /* 8 */ *[color]
- rule = rules[8];
+ rule = rules.get(8);
assertFalse(rule.matches(a));
assertTrue(rule.matches(b));
assertTrue(rule.matches(c));
@@ -126,7 +127,7 @@
assertTrue(rule.matches(e));
// /* 9 */ a[color]
- rule = rules[9];
+ rule = rules.get(9);
assertFalse(rule.matches(a));
assertFalse(rule.matches(b));
assertFalse(rule.matches(c));
@@ -134,7 +135,7 @@
assertFalse(rule.matches(e));
// /* 10 */ b[color]
- rule = rules[10];
+ rule = rules.get(10);
assertFalse(rule.matches(a));
assertTrue(rule.matches(b));
assertFalse(rule.matches(c));
@@ -142,7 +143,7 @@
assertFalse(rule.matches(e));
// /* 11 */ c[color]
- rule = rules[11];
+ rule = rules.get(11);
assertFalse(rule.matches(a));
assertFalse(rule.matches(b));
assertTrue(rule.matches(c));
@@ -150,7 +151,7 @@
assertFalse(rule.matches(e));
// /* 12 */ d[color]
- rule = rules[12];
+ rule = rules.get(12);
assertFalse(rule.matches(a));
assertFalse(rule.matches(b));
assertFalse(rule.matches(c));
@@ -158,7 +159,7 @@
assertFalse(rule.matches(e));
// /* 13 */ *[color=blue]
- rule = rules[13];
+ rule = rules.get(13);
assertFalse(rule.matches(a));
assertTrue(rule.matches(b));
assertFalse(rule.matches(c));
@@ -166,7 +167,7 @@
assertFalse(rule.matches(e));
// /* 14 */ a[color=blue]
- rule = rules[14];
+ rule = rules.get(14);
assertFalse(rule.matches(a));
assertFalse(rule.matches(b));
assertFalse(rule.matches(c));
@@ -174,7 +175,7 @@
assertFalse(rule.matches(e));
// /* 15 */ b[color=blue]
- rule = rules[15];
+ rule = rules.get(15);
assertFalse(rule.matches(a));
assertTrue(rule.matches(b));
assertFalse(rule.matches(c));
@@ -182,7 +183,7 @@
assertFalse(rule.matches(e));
// /* 16 */ b[color='blue']
- rule = rules[16];
+ rule = rules.get(16);
assertFalse(rule.matches(a));
assertTrue(rule.matches(b));
assertFalse(rule.matches(c));
@@ -190,7 +191,7 @@
assertFalse(rule.matches(e));
// /* 17 */ b[color="blue"]
- rule = rules[17];
+ rule = rules.get(17);
assertFalse(rule.matches(a));
assertTrue(rule.matches(b));
assertFalse(rule.matches(c));
@@ -198,7 +199,7 @@
assertFalse(rule.matches(e));
// /* 18 */ c[color=blue]
- rule = rules[18];
+ rule = rules.get(18);
assertFalse(rule.matches(a));
assertFalse(rule.matches(b));
assertFalse(rule.matches(c));
@@ -206,7 +207,7 @@
assertFalse(rule.matches(e));
// /* 19 */ a * { }
- rule = rules[19];
+ rule = rules.get(19);
assertFalse(rule.matches(a));
assertTrue(rule.matches(b));
assertTrue(rule.matches(c));
@@ -214,7 +215,7 @@
assertTrue(rule.matches(e));
// /* 20 */ a > * { }
- rule = rules[20];
+ rule = rules.get(20);
assertFalse(rule.matches(a));
assertTrue(rule.matches(b));
assertFalse(rule.matches(c));
@@ -222,7 +223,7 @@
assertFalse(rule.matches(e));
// /* 21 */ a *[color] { }
- rule = rules[21];
+ rule = rules.get(21);
assertFalse(rule.matches(a));
assertTrue(rule.matches(b));
assertTrue(rule.matches(c));
@@ -230,7 +231,7 @@
assertTrue(rule.matches(e));
// /* 22 */ a > *[color] { }
- rule = rules[22];
+ rule = rules.get(22);
assertFalse(rule.matches(a));
assertTrue(rule.matches(b));
assertFalse(rule.matches(c));
@@ -238,7 +239,7 @@
assertFalse(rule.matches(e));
// /* 23 */ *[color~=blue] { }
- rule = rules[23];
+ rule = rules.get(23);
assertFalse(rule.matches(a));
assertTrue(rule.matches(b));
assertTrue(rule.matches(c));
@@ -255,14 +256,14 @@
d.setAttribute("class", "bar");
// /* 24 */ .foo { }
- rule = rules[24];
+ rule = rules.get(24);
assertFalse(rule.matches(a));
assertTrue(rule.matches(b));
assertTrue(rule.matches(c));
assertFalse(rule.matches(d));
// /* 25 */ .foo.bar { }
- rule = rules[25];
+ rule = rules.get(25);
assertFalse(rule.matches(a));
assertFalse(rule.matches(b));
assertTrue(rule.matches(c));