Skip to content

Commit

Permalink
Change AttrValue::Url to AttrValue::ResolvedUrl
Browse files Browse the repository at this point in the history 
There is actually only one attribute that can use that, the one for
<body background>.
  • Loading branch information
@nox
nox committed Oct 15, 2017
1 parent 8b366a7 commit c7b1d30
Show file tree
Hide file tree
Showing 9 changed files with 48 additions and 72 deletions.
28 changes: 8 additions & 20 deletions components/script/dom/element.rs
View file
Expand Up @@ -1306,26 +1306,13 @@ impl Element {
Some(attr) => attr,
None => return DOMString::new(),
};
let value = attr.value();
match *value {
AttrValue::Url(ref value, _) => {
// XXXManishearth this doesn't handle `javascript:` urls properly
let base = document_from_node(self).base_url();
let value = base.join(value)
.map(|parsed| parsed.into_string())
.unwrap_or_else(|_| value.clone());
DOMString::from(value)
},
_ => panic!("attribute value should be AttrValue::Url(..)"),
}
}

pub fn set_url_attribute(&self, local_name: &LocalName, value: DOMString) {
let value = AttrValue::from_url(
document_from_node(self).base_url(),
value.into(),
);
self.set_attribute(local_name, value);
let value = &**attr.value();
// XXXManishearth this doesn't handle `javascript:` urls properly
let base = document_from_node(self).base_url();
let value = base.join(value)
.map(|parsed| parsed.into_string())
.unwrap_or_else(|_| value.to_owned());
DOMString::from(value)
}

pub fn get_string_attribute(&self, local_name: &LocalName) -> DOMString {
Expand All @@ -1334,6 +1321,7 @@ impl Element {
None => DOMString::new(),
}
}

pub fn set_string_attribute(&self, local_name: &LocalName, value: DOMString) {
assert!(*local_name == local_name.to_ascii_lowercase());
self.set_attribute(local_name, AttrValue::String(value.into()));
Expand Down
23 changes: 13 additions & 10 deletions components/script/dom/htmlbodyelement.rs
View file
Expand Up @@ -4,7 +4,6 @@

use cssparser::RGBA;
use dom::attr::Attr;
use dom::bindings::codegen::Bindings::AttrBinding::AttrBinding::AttrMethods;
use dom::bindings::codegen::Bindings::HTMLBodyElementBinding::{self, HTMLBodyElementMethods};
use dom::bindings::codegen::Bindings::WindowBinding::WindowMethods;
use dom::bindings::inheritance::Castable;
Expand Down Expand Up @@ -74,15 +73,16 @@ impl HTMLBodyElementMethods for HTMLBodyElement {
make_legacy_color_setter!(SetText, "text");

// https://html.spec.whatwg.org/multipage/#dom-body-background
fn Background(&self) -> DOMString {
self.upcast::<Element>()
.get_attribute(&ns!(), &local_name!("background"))
.map(|attr| attr.Value())
.unwrap_or_default()
}
make_getter!(Background, "background");

// https://html.spec.whatwg.org/multipage/#dom-body-background
make_url_setter!(SetBackground, "background");
fn SetBackground(&self, input: DOMString) {
let value = AttrValue::from_resolved_url(
&document_from_node(self).base_url(),
input.into(),
);
self.upcast::<Element>().set_attribute(&local_name!("background"), value);
}

// https://html.spec.whatwg.org/multipage/#windoweventhandlers
window_event_handlers!(ForwardToWindow);
Expand Down Expand Up @@ -120,7 +120,7 @@ impl HTMLBodyElementLayoutHelpers for LayoutDom<HTMLBodyElement> {
unsafe {
(*self.upcast::<Element>().unsafe_get())
.get_attr_for_layout(&ns!(), &local_name!("background"))
.and_then(AttrValue::as_url)
.and_then(AttrValue::as_resolved_url)
.cloned()
}
}
Expand Down Expand Up @@ -160,7 +160,10 @@ impl VirtualMethods for HTMLBodyElement {
local_name!("bgcolor") |
local_name!("text") => AttrValue::from_legacy_color(value.into()),
local_name!("background") => {
AttrValue::from_url(document_from_node(self).base_url(), value.into())
AttrValue::from_resolved_url(
&document_from_node(self).base_url(),
value.into(),
)
},
_ => self.super_type().unwrap().parse_plain_attribute(name, value),
}
Expand Down
3 changes: 1 addition & 2 deletions components/script/dom/htmliframeelement.rs
View file
Expand Up @@ -571,7 +571,7 @@ impl HTMLIFrameElementMethods for HTMLIFrameElement {
make_url_getter!(Src, "src");

// https://html.spec.whatwg.org/multipage/#dom-iframe-src
make_url_setter!(SetSrc, "src");
make_setter!(SetSrc, "src");

// https://html.spec.whatwg.org/multipage/#dom-iframe-sandbox
fn Sandbox(&self) -> DomRoot<DOMTokenList> {
Expand Down Expand Up @@ -761,7 +761,6 @@ impl VirtualMethods for HTMLIFrameElement {

fn parse_plain_attribute(&self, name: &LocalName, value: DOMString) -> AttrValue {
match name {
&local_name!("src") => AttrValue::from_url(document_from_node(self).base_url(), value.into()),
&local_name!("sandbox") => AttrValue::from_serialized_tokenlist(value.into()),
&local_name!("width") => AttrValue::from_dimension(value.into()),
&local_name!("height") => AttrValue::from_dimension(value.into()),
Expand Down
3 changes: 1 addition & 2 deletions components/script/dom/htmlimageelement.rs
View file
Expand Up @@ -826,7 +826,7 @@ impl HTMLImageElementMethods for HTMLImageElement {
make_url_getter!(Src, "src");

// https://html.spec.whatwg.org/multipage/#dom-img-src
make_url_setter!(SetSrc, "src");
make_setter!(SetSrc, "src");

// https://html.spec.whatwg.org/multipage/#dom-img-crossOrigin
fn GetCrossOrigin(&self) -> Option<DOMString> {
Expand Down Expand Up @@ -981,7 +981,6 @@ impl VirtualMethods for HTMLImageElement {

fn parse_plain_attribute(&self, name: &LocalName, value: DOMString) -> AttrValue {
match name {
&local_name!("src") => AttrValue::from_url(document_from_node(self).base_url(), value.into()),
&local_name!("name") => AttrValue::from_atomic(value.into()),
&local_name!("width") | &local_name!("height") => AttrValue::from_dimension(value.into()),
&local_name!("hspace") | &local_name!("vspace") => AttrValue::from_u32(value.into(), 0),
Expand Down
3 changes: 1 addition & 2 deletions components/script/dom/htmlinputelement.rs
View file
Expand Up @@ -530,7 +530,7 @@ impl HTMLInputElementMethods for HTMLInputElement {
make_url_getter!(Src, "src");

// https://html.spec.whatwg.org/multipage/#dom-input-src
make_url_setter!(SetSrc, "src");
make_setter!(SetSrc, "src");

// https://html.spec.whatwg.org/multipage/#dom-input-step
make_getter!(Step, "step");
Expand Down Expand Up @@ -1056,7 +1056,6 @@ impl VirtualMethods for HTMLInputElement {

fn parse_plain_attribute(&self, name: &LocalName, value: DOMString) -> AttrValue {
match name {
&local_name!("src") => AttrValue::from_url(document_from_node(self).base_url(), value.into()),
&local_name!("accept") => AttrValue::from_comma_separated_tokenlist(value.into()),
&local_name!("name") => AttrValue::from_atomic(value.into()),
&local_name!("size") => AttrValue::from_limited_u32(value.into(), DEFAULT_INPUT_SIZE),
Expand Down
3 changes: 1 addition & 2 deletions components/script/dom/htmllinkelement.rs
View file
Expand Up @@ -208,7 +208,6 @@ impl VirtualMethods for HTMLLinkElement {

fn parse_plain_attribute(&self, name: &LocalName, value: DOMString) -> AttrValue {
match name {
&local_name!("href") => AttrValue::from_url(document_from_node(self).base_url(), value.into()),
&local_name!("rel") => AttrValue::from_serialized_tokenlist(value.into()),
_ => self.super_type().unwrap().parse_plain_attribute(name, value),
}
Expand Down Expand Up @@ -374,7 +373,7 @@ impl HTMLLinkElementMethods for HTMLLinkElement {
make_url_getter!(Href, "href");

// https://html.spec.whatwg.org/multipage/#dom-link-href
make_url_setter!(SetHref, "href");
make_setter!(SetHref, "href");

// https://html.spec.whatwg.org/multipage/#dom-link-rel
make_getter!(Rel, "rel");
Expand Down
10 changes: 1 addition & 9 deletions components/script/dom/htmlmediaelement.rs
View file
Expand Up @@ -46,7 +46,6 @@ use std::collections::VecDeque;
use std::mem;
use std::rc::Rc;
use std::sync::{Arc, Mutex};
use style::attr::AttrValue;
use task_source::TaskSource;
use time::{self, Timespec, Duration};

Expand Down Expand Up @@ -840,7 +839,7 @@ impl HTMLMediaElementMethods for HTMLMediaElement {
make_url_getter!(Src, "src");

// https://html.spec.whatwg.org/multipage/#dom-media-src
make_url_setter!(SetSrc, "src");
make_setter!(SetSrc, "src");

// https://html.spec.whatwg.org/multipage/#dom-media-srcobject
fn GetSrcObject(&self) -> Option<DomRoot<Blob>> {
Expand Down Expand Up @@ -915,13 +914,6 @@ impl VirtualMethods for HTMLMediaElement {
Some(self.upcast::<HTMLElement>() as &VirtualMethods)
}

fn parse_plain_attribute(&self, name: &LocalName, value: DOMString) -> AttrValue {
match name {
&local_name!("src") => AttrValue::from_url(document_from_node(self).base_url(), value.into()),
_ => self.super_type().unwrap().parse_plain_attribute(name, value),
}
}

fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
self.super_type().unwrap().attribute_mutated(attr, mutation);

Expand Down
10 changes: 1 addition & 9 deletions components/script/dom/htmlscriptelement.rs
View file
Expand Up @@ -42,7 +42,6 @@ use std::io::{Read, Write};
use std::path::PathBuf;
use std::process::{Command, Stdio};
use std::sync::{Arc, Mutex};
use style::attr::AttrValue;
use style::str::{HTML_SPACE_CHARACTERS, StaticStringVec};
use uuid::Uuid;

Expand Down Expand Up @@ -654,13 +653,6 @@ impl VirtualMethods for HTMLScriptElement {
Some(self.upcast::<HTMLElement>() as &VirtualMethods)
}

fn parse_plain_attribute(&self, name: &LocalName, value: DOMString) -> AttrValue {
match name {
&local_name!("src") => AttrValue::from_url(document_from_node(self).base_url(), value.into()),
_ => self.super_type().unwrap().parse_plain_attribute(name, value),
}
}

fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
self.super_type().unwrap().attribute_mutated(attr, mutation);
match *attr.local_name() {
Expand Down Expand Up @@ -712,7 +704,7 @@ impl HTMLScriptElementMethods for HTMLScriptElement {
make_url_getter!(Src, "src");

// https://html.spec.whatwg.org/multipage/#dom-script-src
make_url_setter!(SetSrc, "src");
make_setter!(SetSrc, "src");

// https://html.spec.whatwg.org/multipage/#dom-script-type
make_getter!(Type, "type");
Expand Down
37 changes: 21 additions & 16 deletions components/style/attr.rs
View file
Expand Up @@ -46,7 +46,12 @@ pub enum AttrValue {
Length(String, Option<Length>),
Color(String, Option<RGBA>),
Dimension(String, LengthOrPercentageOrAuto),
Url(String, Option<ServoUrl>),

/// Stores a URL, computed from the input string and a document's base URL.
///
/// The URL is resolved at setting-time, so this kind of attribute value is
/// not actually suitable for most URL-reflecting IDL attributes.
ResolvedUrl(String, Option<ServoUrl>),

/// Note that this variant is only used transitively as a fast path to set
/// the property declaration block relevant to the style of an element when
Expand Down Expand Up @@ -227,9 +232,9 @@ impl AttrValue {
AttrValue::Atom(value)
}

pub fn from_url(base: ServoUrl, url: String) -> AttrValue {
pub fn from_resolved_url(base: &ServoUrl, url: String) -> AttrValue {
let joined = base.join(&url).ok();
AttrValue::Url(url, joined)
AttrValue::ResolvedUrl(url, joined)
}

pub fn from_legacy_color(string: String) -> AttrValue {
Expand Down Expand Up @@ -307,14 +312,14 @@ impl AttrValue {
}
}

/// Assumes the `AttrValue` is a `Url` and returns its value
/// Assumes the `AttrValue` is a `ResolvedUrl` and returns its value.
///
/// ## Panics
///
/// Panics if the `AttrValue` is not a `Url`
pub fn as_url(&self) -> Option<&ServoUrl> {
/// Panics if the `AttrValue` is not a `ResolvedUrl`
pub fn as_resolved_url(&self) -> Option<&ServoUrl> {
match *self {
AttrValue::Url(_, ref url) => url.as_ref(),
AttrValue::ResolvedUrl(_, ref url) => url.as_ref(),
_ => panic!("Url not found"),
}
}
Expand Down Expand Up @@ -365,15 +370,15 @@ impl ::std::ops::Deref for AttrValue {
fn deref(&self) -> &str {
match *self {
AttrValue::String(ref value) |
AttrValue::TokenList(ref value, _) |
AttrValue::UInt(ref value, _) |
AttrValue::Double(ref value, _) |
AttrValue::Length(ref value, _) |
AttrValue::Color(ref value, _) |
AttrValue::Int(ref value, _) |
AttrValue::Url(ref value, _) |
AttrValue::Declaration(ref value, _) |
AttrValue::Dimension(ref value, _) => &value,
AttrValue::TokenList(ref value, _) |
AttrValue::UInt(ref value, _) |
AttrValue::Double(ref value, _) |
AttrValue::Length(ref value, _) |
AttrValue::Color(ref value, _) |
AttrValue::Int(ref value, _) |
AttrValue::ResolvedUrl(ref value, _) |
AttrValue::Declaration(ref value, _) |
AttrValue::Dimension(ref value, _) => &value,
AttrValue::Atom(ref value) => &value,
}
}
Expand Down

0 comments on commit c7b1d30

Please sign in to comment.