Address review comments.

This commit is contained in:
Josh Matthews 2014-12-12 12:58:42 -05:00
parent 9d82e06e64
commit 3cfe8ab53e
10 changed files with 92 additions and 121 deletions

View file

@ -63,103 +63,95 @@ impl<'a> CSSStyleDeclarationMethods for JSRef<'a, CSSStyleDeclaration> {
fn Length(self) -> u32 { fn Length(self) -> u32 {
let owner = self.owner.root(); let owner = self.owner.root();
let elem: JSRef<Element> = ElementCast::from_ref(*owner); let elem: JSRef<Element> = ElementCast::from_ref(*owner);
let style_attribute = elem.style_attribute().borrow(); let len = match *elem.style_attribute().borrow() {
style_attribute.as_ref().map(|declarations| { Some(ref declarations) => declarations.normal.len() + declarations.important.len(),
declarations.normal.len() + declarations.important.len() None => 0
}).unwrap_or(0) as u32 };
len as u32
} }
fn Item(self, index: u32) -> DOMString { fn Item(self, index: u32) -> DOMString {
let owner = self.owner.root(); let owner = self.owner.root();
let elem: JSRef<Element> = ElementCast::from_ref(*owner); let elem: JSRef<Element> = ElementCast::from_ref(*owner);
let style_attribute = elem.style_attribute().borrow(); let style_attribute = elem.style_attribute().borrow();
style_attribute.as_ref().and_then(|declarations| { let result = style_attribute.as_ref().and_then(|declarations| {
if index as uint > declarations.normal.len() { if index as uint > declarations.normal.len() {
declarations.important declarations.important
.iter() .as_slice()
.nth(index as uint - declarations.normal.len()) .get(index as uint - declarations.normal.len())
.map(|decl| format!("{} !important", decl)) .map(|decl| format!("{} !important", decl))
} else { } else {
declarations.normal declarations.normal
.iter() .as_slice()
.nth(index as uint) .get(index as uint)
.map(|decl| format!("{}", decl)) .map(|decl| format!("{}", decl))
} }
}).unwrap_or("".to_string()) });
result.unwrap_or("".to_string())
} }
//http://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertyvalue // http://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertyvalue
fn GetPropertyValue(self, property: DOMString) -> DOMString { fn GetPropertyValue(self, property: DOMString) -> DOMString {
// 1. Let property be property converted to ASCII lowercase. // Step 1
let property = Atom::from_slice(property.as_slice().to_ascii_lower().as_slice()); let property = Atom::from_slice(property.as_slice().to_ascii_lower().as_slice());
// 2. If property is a shorthand property, then follow these substeps: // Step 2
let longhand_properties = longhands_from_shorthand(property.as_slice()); let longhand_properties = longhands_from_shorthand(property.as_slice());
if longhand_properties.is_some() { if let Some(longhand_properties) = longhand_properties {
// 1. Let list be a new empty array. // Step 2.1
let mut list = vec!(); let mut list = vec!();
// 2. For each longhand property longhand that property maps to, in canonical order, // Step 2.2
// follow these substeps: for longhand in longhand_properties.iter() {
for longhand in longhand_properties.unwrap().iter() { // Step 2.2.1
// 1. If longhand is a case-sensitive match for a property name of a
// CSS declaration in the declarations, let declaration be that CSS
// declaration, or null otherwise.
let declaration = self.get_declaration(&Atom::from_slice(longhand.as_slice())); let declaration = self.get_declaration(&Atom::from_slice(longhand.as_slice()));
// 2. If declaration is null, return the empty string and terminate these // Step 2.2.2 & 2.2.3
// steps.
match declaration { match declaration {
// 3. Append the declaration to list.
Some(declaration) => list.push(declaration), Some(declaration) => list.push(declaration),
None => return "".to_string(), None => return "".to_string(),
} }
} }
// 3. Return the serialization of list and terminate these steps. // Step 2.3
return serialize_list(&list); return serialize_list(&list);
} }
// 3. If property is a case-sensitive match for a property name of a CSS declaration // Step 3 & 4
// in the declarations, return the result of invoking serialize a CSS value of that if let Some(ref declaration) = self.get_declaration(&property) {
// declaration and terminate these steps. serialize_value(declaration)
// 4. Return the empty string. } else {
let declaration = self.get_declaration(&property); "".to_string()
declaration.as_ref() }
.map(|declaration| serialize_value(declaration))
.unwrap_or("".to_string())
} }
// http://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-setproperty
fn SetProperty(self, property: DOMString, value: DOMString, fn SetProperty(self, property: DOMString, value: DOMString,
priority: DOMString) -> ErrorResult { priority: DOMString) -> ErrorResult {
// 1. If the readonly flag is set, throw a NoModificationAllowedError exception //TODO: disallow modifications if readonly flag is set
// and terminate these steps.
//TODO
// 2. Let property be property converted to ASCII lowercase. // Step 2
let property = property.as_slice().to_ascii_lower(); let property = property.as_slice().to_ascii_lower();
// 3. If property is not a case-sensitive match for a supported CSS property, // Step 3
// terminate this algorithm.
if !is_supported_property(property.as_slice()) { if !is_supported_property(property.as_slice()) {
return Ok(()); return Ok(());
} }
// 4. If value is the empty string, invoke removeProperty() with property as argument // Step 4
// and terminate this algorithm.
if value.is_empty() { if value.is_empty() {
self.RemoveProperty(property.clone()); self.RemoveProperty(property);
return Ok(()); return Ok(());
} }
// 5. If priority is not the empty string and is not an ASCII case-insensitive match // Step 5
// for the string "important", terminate this algorithm.
let priority = priority.as_slice().to_ascii_lower(); let priority = priority.as_slice().to_ascii_lower();
if priority.as_slice() != "!important" && !priority.is_empty() { if priority.as_slice() != "!important" && !priority.is_empty() {
return Ok(()); return Ok(());
} }
// 6. Let `component value list` be the result of parsing value for property `property`. // Step 6
let mut synthesized_declaration = String::from_str(property.as_slice()); let mut synthesized_declaration = String::from_str(property.as_slice());
synthesized_declaration.push_str(": "); synthesized_declaration.push_str(": ");
synthesized_declaration.push_str(value.as_slice()); synthesized_declaration.push_str(value.as_slice());
@ -170,7 +162,7 @@ impl<'a> CSSStyleDeclarationMethods for JSRef<'a, CSSStyleDeclaration> {
let decl_block = parse_style_attribute(synthesized_declaration.as_slice(), let decl_block = parse_style_attribute(synthesized_declaration.as_slice(),
&page.get_url()); &page.get_url());
// 7. If `component value list` is null terminate these steps. // Step 7
if decl_block.normal.len() == 0 { if decl_block.normal.len() == 0 {
return Ok(()); return Ok(());
} }
@ -178,19 +170,9 @@ impl<'a> CSSStyleDeclarationMethods for JSRef<'a, CSSStyleDeclaration> {
let owner = self.owner.root(); let owner = self.owner.root();
let element: JSRef<Element> = ElementCast::from_ref(*owner); let element: JSRef<Element> = ElementCast::from_ref(*owner);
//XXXjdm https://www.w3.org/Bugs/Public/show_bug.cgi?id=27589 // Step 8
assert!(decl_block.important.len() == 0);
for decl in decl_block.normal.iter() { for decl in decl_block.normal.iter() {
// 8. If property is a shorthand property, then for each longhand property // Step 9
// longhand that property maps to, in canonical order, set the CSS
// declaration value longhand to the appropriate value(s) from component
// value list, and with the list of declarations being the declarations.
// 9. Otherwise, set the CSS declaration value property to the
// value component value list, and with the list of declarations
// being the declarations.
element.update_inline_style(decl.clone(), !priority.is_empty()); element.update_inline_style(decl.clone(), !priority.is_empty());
} }
@ -200,49 +182,48 @@ impl<'a> CSSStyleDeclarationMethods for JSRef<'a, CSSStyleDeclaration> {
Ok(()) Ok(())
} }
// http://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-setpropertyvalue
fn SetPropertyValue(self, property: DOMString, value: DOMString) -> ErrorResult { fn SetPropertyValue(self, property: DOMString, value: DOMString) -> ErrorResult {
self.SetProperty(property, value, "".to_string()) self.SetProperty(property, value, "".to_string())
} }
// http://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-removeproperty
fn RemoveProperty(self, property: DOMString) -> DOMString { fn RemoveProperty(self, property: DOMString) -> DOMString {
// 1. If the readonly flag is set, throw a NoModificationAllowedError exception //TODO: disallow modifications if readonly flag is set
// and terminate these steps.
//TODO
// 2. Let `property` be property converted to ASCII lowercase. // Step 2
let property = property.as_slice().to_ascii_lower(); let property = property.as_slice().to_ascii_lower();
// 3. Let `value` be the return value of invoking getPropertyValue() with `property` // Step 3
// as argument.
let value = self.GetPropertyValue(property.clone()); let value = self.GetPropertyValue(property.clone());
// 4. If `property` is a shorthand property, for each longhand property `longhand` that
// `property` maps to, invoke removeProperty() with `longhand` as argument.
let longhand_properties = longhands_from_shorthand(property.as_slice()); let longhand_properties = longhands_from_shorthand(property.as_slice());
match longhand_properties { match longhand_properties {
Some(longhands) => { Some(longhands) => {
// Step 4
for longhand in longhands.iter() { for longhand in longhands.iter() {
self.RemoveProperty(longhand.clone()); self.RemoveProperty(longhand.clone());
} }
} }
// 5. Otherwise, if `property` is a case-sensitive match for a property name of a
// CSS declaration in the declarations, remove that CSS declaration.
None => { None => {
// Step 5
let owner = self.owner.root(); let owner = self.owner.root();
let elem: JSRef<Element> = ElementCast::from_ref(*owner); let elem: JSRef<Element> = ElementCast::from_ref(*owner);
elem.remove_inline_style_property(property) elem.remove_inline_style_property(property)
} }
} }
// 6. Return value. // Step 6
value value
} }
// http://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-cssfloat
fn CssFloat(self) -> DOMString { fn CssFloat(self) -> DOMString {
self.GetPropertyValue("float".to_string()) self.GetPropertyValue("float".to_string())
} }
// http://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-cssfloat
fn SetCssFloat(self, value: DOMString) -> ErrorResult { fn SetCssFloat(self, value: DOMString) -> ErrorResult {
self.SetPropertyValue("float".to_string(), value) self.SetPropertyValue("float".to_string(), value)
} }

View file

@ -528,8 +528,6 @@ impl<'a> ElementHelpers<'a> for JSRef<'a, Element> {
} }
fn remove_inline_style_property(self, property: DOMString) { fn remove_inline_style_property(self, property: DOMString) {
//FIXME: Rust bug incorrectly thinks inline_declarations doesn't need mut,
// and allow(unused_mut) on this method does nothing.
let mut inline_declarations = self.style_attribute.borrow_mut(); let mut inline_declarations = self.style_attribute.borrow_mut();
inline_declarations.as_mut().map(|declarations| { inline_declarations.as_mut().map(|declarations| {
let index = declarations.normal let index = declarations.normal
@ -557,47 +555,44 @@ impl<'a> ElementHelpers<'a> for JSRef<'a, Element> {
} }
fn update_inline_style(self, property_decl: style::PropertyDeclaration, important: bool) { fn update_inline_style(self, property_decl: style::PropertyDeclaration, important: bool) {
//FIXME: Rust bug incorrectly thinks inline_declarations doesn't need mut, let mut inline_declarations = self.style_attribute().borrow_mut();
// and allow(unused_mut) on this method does nothing. if let Some(ref mut declarations) = *inline_declarations.deref_mut() {
let mut inline_declarations = self.style_attribute.borrow_mut(); let existing_declarations = if important {
let exists = inline_declarations.is_some(); declarations.important.make_unique()
if exists {
let declarations = inline_declarations.deref_mut().as_mut().unwrap();
let mut existing_declarations = if important {
declarations.important.clone()
} else { } else {
declarations.normal.clone() declarations.normal.make_unique()
}; };
for declaration in existing_declarations.make_unique().iter_mut() {
for declaration in existing_declarations.iter_mut() {
if declaration.name() == property_decl.name() { if declaration.name() == property_decl.name() {
*declaration = property_decl; *declaration = property_decl;
return; return;
} }
} }
existing_declarations.make_unique().push(property_decl); existing_declarations.push(property_decl);
} else { return;
let (important, normal) = if important {
(vec!(property_decl), vec!())
} else {
(vec!(), vec!(property_decl))
};
*inline_declarations = Some(style::PropertyDeclarationBlock {
important: Arc::new(important),
normal: Arc::new(normal),
});
} }
let (important, normal) = if important {
(vec!(property_decl), vec!())
} else {
(vec!(), vec!(property_decl))
};
*inline_declarations = Some(style::PropertyDeclarationBlock {
important: Arc::new(important),
normal: Arc::new(normal),
});
} }
fn get_inline_style_declaration(self, property: &Atom) -> Option<style::PropertyDeclaration> { fn get_inline_style_declaration(self, property: &Atom) -> Option<style::PropertyDeclaration> {
let mut inline_declarations = self.style_attribute.borrow(); let inline_declarations = self.style_attribute.borrow();
inline_declarations.as_ref().and_then(|declarations| { inline_declarations.as_ref().and_then(|declarations| {
for declaration in declarations.normal.iter().chain(declarations.important.iter()) { declarations.normal
if declaration.matches(property.as_slice()) { .iter()
return Some(declaration.clone()); .chain(declarations.important.iter())
} .find(|decl| decl.matches(property.as_slice()))
} .map(|decl| decl.clone())
None
}) })
} }
} }

View file

@ -72,13 +72,11 @@ impl<'a> PrivateHTMLElementHelpers for JSRef<'a, HTMLElement> {
impl<'a> HTMLElementMethods for JSRef<'a, HTMLElement> { impl<'a> HTMLElementMethods for JSRef<'a, HTMLElement> {
fn Style(self) -> Temporary<CSSStyleDeclaration> { fn Style(self) -> Temporary<CSSStyleDeclaration> {
if self.style_decl.get().is_none() { self.style_decl.or_init(|| {
let global = window_from_node(self).root(); let global = window_from_node(self).root();
let style_props = CSS2Properties::new(*global, self).root(); let style_props = CSS2Properties::new(*global, self);
let style_decl: JSRef<CSSStyleDeclaration> = CSSStyleDeclarationCast::from_ref(*style_props); CSSStyleDeclarationCast::from_temporary(style_props)
self.style_decl.assign(Some(style_decl)); })
}
self.style_decl.get().unwrap()
} }
make_getter!(Title) make_getter!(Title)

View file

@ -194,7 +194,7 @@ pub struct SharedLayoutData {
/// Encapsulates the abstract layout data. /// Encapsulates the abstract layout data.
pub struct LayoutData { pub struct LayoutData {
chan: Option<LayoutChan>, chan: Option<LayoutChan>,
pub shared_data: SharedLayoutData, _shared_data: SharedLayoutData,
_data: *const (), _data: *const (),
} }

View file

@ -4,7 +4,8 @@
* You can obtain one at http://mozilla.org/MPL/2.0/. * You can obtain one at http://mozilla.org/MPL/2.0/.
* *
* The origin of this IDL file is * The origin of this IDL file is
* http://dev.w3.org/csswg/cssom/ * http://mxr.mozilla.org/mozilla-central/source/dom/webidl/CSS2Properties.webidl.in
* http://mxr.mozilla.org/mozilla-central/source/dom/bindings/GenerateCSS2PropertiesWebIDL.py
* *
*/ */

View file

@ -23,8 +23,7 @@ interface CSSStyleDeclaration {
//[Throws] //[Throws]
//void setPropertyPriority(DOMString property, [TreatNullAs=EmptyString] DOMString priority); //void setPropertyPriority(DOMString property, [TreatNullAs=EmptyString] DOMString priority);
DOMString removeProperty(DOMString property); DOMString removeProperty(DOMString property);
// Not implemented yet: //readonly attribute CSSRule? parentRule;
// readonly attribute CSSRule? parentRule;
[SetterThrows] [SetterThrows]
attribute DOMString cssFloat; attribute DOMString cssFloat;
}; };

View file

@ -5,7 +5,7 @@
#![comment = "The Servo Parallel Browser Project"] #![comment = "The Servo Parallel Browser Project"]
#![license = "MPL"] #![license = "MPL"]
#![feature(default_type_params, globs, macro_rules, phase, unsafe_destructor)] #![feature(default_type_params, globs, macro_rules, phase, unsafe_destructor, if_let)]
#![deny(unused_imports)] #![deny(unused_imports)]
#![deny(unused_variables)] #![deny(unused_variables)]

View file

@ -12,17 +12,17 @@ function expect(num) {
function _fail(s, m) { function _fail(s, m) {
_tests++; _tests++;
// string split to avoid problems with tests that end up printing the value of window._fail. // string split to avoid problems with tests that end up printing the value of window._fail.
console.log(_oneline("TEST-UNEXPECTED" + "-FAIL | " + s + ": " + m)); window.alert(_oneline("TEST-UNEXPECTED" + "-FAIL | " + s + ": " + m));
} }
function _pass(s, m) { function _pass(s, m) {
_tests++; _tests++;
//console.log(_oneline("TEST-PASS | " + s + ": " + m)); window.alert(_oneline("TEST-PASS | " + s + ": " + m));
} }
function _printer(opstr, op) { function _printer(opstr, op) {
return function (a, b, msg) { return function (a, b, msg) {
var f = op(a,b) ? _pass : _fail; let f = op(a,b) ? _pass : _fail;
if (!msg) msg = ""; if (!msg) msg = "";
f(a + " " + opstr + " " + b, msg); f(a + " " + opstr + " " + b, msg);
}; };

View file

@ -3,10 +3,8 @@ function run_tests(properties) {
var name = Object.keys(properties)[property]; var name = Object.keys(properties)[property];
var generator = create_value_generator(properties[name]); var generator = create_value_generator(properties[name]);
var prop = properties[name].property || name; var prop = properties[name].property || name;
//setTimeout(function(name, generator, prop) { while (run_test(name, generator, prop)) {
while (run_test(name, generator, prop)) { }
}
//}, 0, name, generator, prop);
} }
} }
@ -85,7 +83,6 @@ function to_idl(property) {
} }
function run_test(property, generator, prop) { function run_test(property, generator, prop) {
//console.log("testing " + property + ' ' + to_idl(property));
var elem = document.createElement('div'); var elem = document.createElement('div');
document.getElementById('parent').appendChild(elem); document.getElementById('parent').appendChild(elem);
var style = generate_inline_style(property, generator()); var style = generate_inline_style(property, generator());

View file

@ -1,9 +1,9 @@
<div id="test" style="display: block; background-color: black; background-position: top left; font-style: italic">test text!</div> <div id="test" style="display: block; background-color: black; background-position: top left; font-style: italic">test text!</div>
<script> <script>
var id = document.getElementById('test'); var id = document.getElementById('test');
/*alert(id.style.display); alert(id.style.display);
id.style.display = 'none'; id.style.display = 'none';
alert(id.style.display);*/ alert(id.style.display);
alert(id.style.fontStyle + ' ' + id.style['font-style']); alert(id.style.fontStyle + ' ' + id.style['font-style']);
id.style.background = "black"; id.style.background = "black";
alert(document.getElementById('test').style.background); alert(document.getElementById('test').style.background);