Do not lowercase names for *AttributeNS APIs. (Move lower-casing to callers.)

This commit is contained in:
Simon Sapin 2014-01-25 11:25:42 -08:00
parent d6b5015d7a
commit b8556afeeb
4 changed files with 44 additions and 22 deletions

View file

@ -14,11 +14,13 @@
//! (2) Layout is not allowed to see anything with `Abstract` in the name, because it could hang //! (2) Layout is not allowed to see anything with `Abstract` in the name, because it could hang
//! onto these objects and cause use-after-free. //! onto these objects and cause use-after-free.
use std::ascii::StrAsciiExt;
use extra::url::Url; use extra::url::Url;
use script::dom::element::{Element, HTMLAreaElementTypeId, HTMLAnchorElementTypeId}; use script::dom::element::{Element, HTMLAreaElementTypeId, HTMLAnchorElementTypeId};
use script::dom::element::{HTMLLinkElementTypeId}; use script::dom::element::{HTMLLinkElementTypeId};
use script::dom::htmliframeelement::HTMLIFrameElement; use script::dom::htmliframeelement::HTMLIFrameElement;
use script::dom::htmlimageelement::HTMLImageElement; use script::dom::htmlimageelement::HTMLImageElement;
use script::dom::namespace;
use script::dom::namespace::Namespace; use script::dom::namespace::Namespace;
use script::dom::node::{AbstractNode, DocumentNodeTypeId, ElementNodeTypeId, Node, NodeTypeId}; use script::dom::node::{AbstractNode, DocumentNodeTypeId, ElementNodeTypeId, Node, NodeTypeId};
use script::dom::text::Text; use script::dom::text::Text;
@ -246,7 +248,7 @@ impl<'ln> TNode<LayoutElement<'ln>> for LayoutNode<'ln> {
self.node.node().prev_sibling.map(|node| self.new_with_this_lifetime(node)) self.node.node().prev_sibling.map(|node| self.new_with_this_lifetime(node))
} }
} }
fn next_sibling(&self) -> Option<LayoutNode<'ln>> { fn next_sibling(&self) -> Option<LayoutNode<'ln>> {
unsafe { unsafe {
self.node.node().next_sibling.map(|node| self.new_with_this_lifetime(node)) self.node.node().next_sibling.map(|node| self.new_with_this_lifetime(node))
@ -390,6 +392,20 @@ impl<'le> TElement for LayoutElement<'le> {
unsafe { self.element.get_attr_val_for_layout(namespace, name) } unsafe { self.element.get_attr_val_for_layout(namespace, name) }
} }
fn match_attr(&self, ns_url: Option<~str>, name: &str, test: &|&str| -> bool) -> bool {
let temp;
let name = if self.element.html_element_in_html_document() {
temp = name.to_ascii_lower();
temp.as_slice()
} else {
name
};
match self.get_attr(ns_url, name) {
Some(value) => (*test)(value),
None => false,
}
}
fn get_link(&self) -> Option<~str> { fn get_link(&self) -> Option<~str> {
// FIXME: This is HTML only. // FIXME: This is HTML only.
match self.element.node.type_id { match self.element.node.type_id {
@ -398,7 +414,8 @@ impl<'le> TElement for LayoutElement<'le> {
ElementNodeTypeId(HTMLAnchorElementTypeId) | ElementNodeTypeId(HTMLAnchorElementTypeId) |
ElementNodeTypeId(HTMLAreaElementTypeId) | ElementNodeTypeId(HTMLAreaElementTypeId) |
ElementNodeTypeId(HTMLLinkElementTypeId) => { ElementNodeTypeId(HTMLLinkElementTypeId) => {
self.get_attr(None, "href").map(|val| val.to_owned()) unsafe { self.element.get_attr_val_for_layout(namespace::Null, "href") }
.map(|val| val.to_owned())
} }
_ => None, _ => None,
} }

View file

@ -7,7 +7,7 @@
use dom::attr::Attr; use dom::attr::Attr;
use dom::attrlist::AttrList; use dom::attrlist::AttrList;
use dom::bindings::utils::{Reflectable, DOMString, ErrorResult, Fallible, Reflector}; use dom::bindings::utils::{Reflectable, DOMString, ErrorResult, Fallible, Reflector};
use dom::bindings::utils::{null_str_as_empty, NamespaceError}; use dom::bindings::utils::NamespaceError;
use dom::bindings::utils::{InvalidCharacter, QName, Name, InvalidXMLName, xml_name_type}; use dom::bindings::utils::{InvalidCharacter, QName, Name, InvalidXMLName, xml_name_type};
use dom::htmlcollection::HTMLCollection; use dom::htmlcollection::HTMLCollection;
use dom::clientrect::ClientRect; use dom::clientrect::ClientRect;
@ -137,30 +137,23 @@ impl Element {
} }
} }
pub fn normalize_attr_name(&self, name: Option<DOMString>) -> ~str { pub fn html_element_in_html_document(&self) -> bool {
//FIXME: Throw for XML-invalid names
let owner = self.node.owner_doc(); let owner = self.node.owner_doc();
if owner.document().doctype == document::HTML { // && self.namespace == Namespace::HTML self.namespace == namespace::HTML &&
null_str_as_empty(&name).to_ascii_lower() // FIXME: check that this matches what the spec calls "is in an HTML document"
} else { owner.document().doctype == document::HTML
null_str_as_empty(&name)
}
} }
pub fn get_attribute(&self, pub fn get_attribute(&self,
namespace: Namespace, namespace: Namespace,
name: &str) -> Option<@mut Attr> { name: &str) -> Option<@mut Attr> {
// FIXME: only case-insensitive in the HTML namespace (as opposed to SVG, etc.)
let name = name.to_ascii_lower();
self.attrs.iter().find(|attr| { self.attrs.iter().find(|attr| {
name == attr.local_name && attr.namespace == namespace name == attr.local_name && attr.namespace == namespace
}).map(|&x| x) }).map(|&x| x)
} }
pub unsafe fn get_attr_val_for_layout(&self, namespace: Namespace, name: &str) pub unsafe fn get_attr_val_for_layout(&self, namespace: Namespace, name: &str)
-> Option<&'static str> { -> Option<&'static str> {
// FIXME: only case-insensitive in the HTML namespace (as opposed to SVG, etc.)
let name = name.to_ascii_lower();
self.attrs.iter().find(|attr: & &@mut Attr| { self.attrs.iter().find(|attr: & &@mut Attr| {
// unsafely avoid a borrow because this is accessed by many tasks // unsafely avoid a borrow because this is accessed by many tasks
// during parallel layout // during parallel layout
@ -174,8 +167,6 @@ impl Element {
pub fn set_attr(&mut self, abstract_self: AbstractNode, name: DOMString, value: DOMString) pub fn set_attr(&mut self, abstract_self: AbstractNode, name: DOMString, value: DOMString)
-> ErrorResult { -> ErrorResult {
// FIXME: HTML-in-HTML only.
let name = name.to_ascii_lower();
self.set_attribute(abstract_self, namespace::Null, name, value) self.set_attribute(abstract_self, namespace::Null, name, value)
} }
@ -402,6 +393,11 @@ impl Element {
} }
pub fn GetAttribute(&self, name: DOMString) -> Option<DOMString> { pub fn GetAttribute(&self, name: DOMString) -> Option<DOMString> {
let name = if self.html_element_in_html_document() {
name.to_ascii_lower()
} else {
name
};
self.get_attribute(Null, name).map(|s| s.Value()) self.get_attribute(Null, name).map(|s| s.Value())
} }
@ -413,6 +409,12 @@ impl Element {
pub fn SetAttribute(&mut self, abstract_self: AbstractNode, name: DOMString, value: DOMString) pub fn SetAttribute(&mut self, abstract_self: AbstractNode, name: DOMString, value: DOMString)
-> ErrorResult { -> ErrorResult {
// FIXME: If name does not match the Name production in XML, throw an "InvalidCharacterError" exception.
let name = if self.html_element_in_html_document() {
name.to_ascii_lower()
} else {
name
};
self.set_attr(abstract_self, name, value) self.set_attr(abstract_self, name, value)
} }
@ -435,6 +437,11 @@ impl Element {
pub fn RemoveAttribute(&mut self, pub fn RemoveAttribute(&mut self,
abstract_self: AbstractNode, abstract_self: AbstractNode,
name: DOMString) -> ErrorResult { name: DOMString) -> ErrorResult {
let name = if self.html_element_in_html_document() {
name.to_ascii_lower()
} else {
name
};
self.remove_attribute(abstract_self, namespace::Null, name) self.remove_attribute(abstract_self, namespace::Null, name)
} }

View file

@ -19,6 +19,7 @@ pub trait TNode<E:TElement> : Clone {
pub trait TElement { pub trait TElement {
fn get_attr(&self, namespace: Option<~str>, attr: &str) -> Option<&'static str>; fn get_attr(&self, namespace: Option<~str>, attr: &str) -> Option<&'static str>;
fn match_attr(&self, ns_url: Option<~str>, name: &str, test: &|&str| -> bool) -> bool;
fn get_link(&self) -> Option<~str>; fn get_link(&self) -> Option<~str>;
fn get_local_name<'a>(&'a self) -> &'a str; fn get_local_name<'a>(&'a self) -> &'a str;
fn get_namespace_url<'a>(&'a self) -> &'a str; fn get_namespace_url<'a>(&'a self) -> &'a str;

View file

@ -701,14 +701,11 @@ fn match_attribute<E:TElement,
N:TNode<E>>( N:TNode<E>>(
attr: &AttrSelector, attr: &AttrSelector,
element: &N, element: &N,
f: |&str| -> bool) test: |&str| -> bool)
-> bool { -> bool {
element.with_element(|element: &E| { element.with_element(|element: &E| {
// FIXME: avoid .clone() here? See #1367 // FIXME: avoid .clone() here? See #1367
match element.get_attr(attr.namespace.clone(), attr.name) { element.match_attr(attr.namespace.clone(), attr.name, &test)
None => false,
Some(value) => f(value)
}
}) })
} }