Skip to content

Commit

Permalink
Do not lowercase names for *AttributeNS APIs. (Move lower-casing to c…
Browse files Browse the repository at this point in the history
…allers.)
  • Loading branch information
SimonSapin authored and therealglazou committed Feb 20, 2014
1 parent 47ee201 commit c6bb5eb
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 22 deletions.
21 changes: 19 additions & 2 deletions src/components/main/layout/wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@
//! (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.

use std::ascii::StrAsciiExt;
use extra::url::Url;
use script::dom::element::{Element, HTMLAreaElementTypeId, HTMLAnchorElementTypeId};
use script::dom::element::{HTMLLinkElementTypeId};
use script::dom::htmliframeelement::HTMLIFrameElement;
use script::dom::htmlimageelement::HTMLImageElement;
use script::dom::namespace;
use script::dom::namespace::Namespace;
use script::dom::node::{AbstractNode, DocumentNodeTypeId, ElementNodeTypeId, Node, NodeTypeId};
use script::dom::text::Text;
Expand Down Expand Up @@ -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))
}
}

fn next_sibling(&self) -> Option<LayoutNode<'ln>> {
unsafe {
self.node.node().next_sibling.map(|node| self.new_with_this_lifetime(node))
Expand Down Expand Up @@ -390,6 +392,20 @@ impl<'le> TElement for LayoutElement<'le> {
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> {
// FIXME: This is HTML only.
match self.element.node.type_id {
Expand All @@ -398,7 +414,8 @@ impl<'le> TElement for LayoutElement<'le> {
ElementNodeTypeId(HTMLAnchorElementTypeId) |
ElementNodeTypeId(HTMLAreaElementTypeId) |
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,
}
Expand Down
37 changes: 22 additions & 15 deletions src/components/script/dom/element.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use dom::attr::Attr;
use dom::attrlist::AttrList;
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::htmlcollection::HTMLCollection;
use dom::clientrect::ClientRect;
Expand Down Expand Up @@ -137,30 +137,23 @@ impl Element {
}
}

pub fn normalize_attr_name(&self, name: Option<DOMString>) -> ~str {
//FIXME: Throw for XML-invalid names
pub fn html_element_in_html_document(&self) -> bool {
let owner = self.node.owner_doc();
if owner.document().doctype == document::HTML { // && self.namespace == Namespace::HTML
null_str_as_empty(&name).to_ascii_lower()
} else {
null_str_as_empty(&name)
}
self.namespace == namespace::HTML &&
// FIXME: check that this matches what the spec calls "is in an HTML document"
owner.document().doctype == document::HTML
}

pub fn get_attribute(&self,
namespace: Namespace,
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| {
name == attr.local_name && attr.namespace == namespace
}).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> {
// 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| {
// unsafely avoid a borrow because this is accessed by many tasks
// during parallel layout
Expand All @@ -174,8 +167,6 @@ impl Element {

pub fn set_attr(&mut self, abstract_self: AbstractNode, name: DOMString, value: DOMString)
-> ErrorResult {
// FIXME: HTML-in-HTML only.
let name = name.to_ascii_lower();
self.set_attribute(abstract_self, namespace::Null, name, value)
}

Expand Down Expand Up @@ -402,6 +393,11 @@ impl Element {
}

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())
}

Expand All @@ -413,6 +409,12 @@ impl Element {

pub fn SetAttribute(&mut self, abstract_self: AbstractNode, name: DOMString, value: DOMString)
-> 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)
}

Expand All @@ -435,6 +437,11 @@ impl Element {
pub fn RemoveAttribute(&mut self,
abstract_self: AbstractNode,
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)
}

Expand Down
1 change: 1 addition & 0 deletions src/components/style/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub trait TNode<E:TElement> : Clone {

pub trait TElement {
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_local_name<'a>(&'a self) -> &'a str;
fn get_namespace_url<'a>(&'a self) -> &'a str;
Expand Down
7 changes: 2 additions & 5 deletions src/components/style/selector_matching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -701,14 +701,11 @@ fn match_attribute<E:TElement,
N:TNode<E>>(
attr: &AttrSelector,
element: &N,
f: |&str| -> bool)
test: |&str| -> bool)
-> bool {
element.with_element(|element: &E| {
// FIXME: avoid .clone() here? See #1367
match element.get_attr(attr.namespace.clone(), attr.name) {
None => false,
Some(value) => f(value)
}
element.match_attr(attr.namespace.clone(), attr.name, &test)
})
}

Expand Down

0 comments on commit c6bb5eb

Please sign in to comment.