Auto merge of #22456 - dlrobertson:fix_src, r=ferjm

Update src/href attributes to be a USVString

The following IDLs have the src/href attributes typed as a `DOMString`
while in the spec the attribute has been updated to be a `USVString`:

 - [HTMLIFrameElement]
 - [HTMLImageElement]
 - [HTMLInputElement]
 - [HTMLLinkElement]
 - [HTMLMediaElement]
 - [HTMLScriptElement]

Fixes: #22454

[HTMLIFrameElement]: https://html.spec.whatwg.org/multipage/#htmliframeelement
[HTMLImageElement]: https://html.spec.whatwg.org/multipage/#htmlimageelement
[HTMLInputElement]: https://html.spec.whatwg.org/multipage/#htmlinputelement
[HTMLLinkElement]: https://html.spec.whatwg.org/multipage/#htmllinkelement
[HTMLMediaElement]: https://html.spec.whatwg.org/multipage/#htmlmediaelement
[HTMLScriptElement]: https://html.spec.whatwg.org/multipage/#htmlscriptelement

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22456)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2018-12-22 23:50:40 -05:00 committed by GitHub
commit 27577955aa
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
16 changed files with 127 additions and 73 deletions

View file

@ -79,9 +79,63 @@ impl ops::Deref for ByteString {
/// A string that is constructed from a UCS-2 buffer by replacing invalid code
/// points with the replacement character.
#[derive(Clone, Default, MallocSizeOf)]
#[derive(Clone, Default, Eq, MallocSizeOf, Ord, PartialEq, PartialOrd)]
pub struct USVString(pub String);
impl Borrow<str> for USVString {
#[inline]
fn borrow(&self) -> &str {
&self.0
}
}
impl Deref for USVString {
type Target = str;
#[inline]
fn deref(&self) -> &str {
&self.0
}
}
impl DerefMut for USVString {
#[inline]
fn deref_mut(&mut self) -> &mut str {
&mut self.0
}
}
impl AsRef<str> for USVString {
fn as_ref(&self) -> &str {
&self.0
}
}
impl fmt::Display for USVString {
#[inline]
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
fmt::Display::fmt(&**self, f)
}
}
impl PartialEq<str> for USVString {
fn eq(&self, other: &str) -> bool {
&**self == other
}
}
impl<'a> PartialEq<&'a str> for USVString {
fn eq(&self, other: &&'a str) -> bool {
&**self == *other
}
}
impl From<String> for USVString {
fn from(contents: String) -> USVString {
USVString(contents)
}
}
/// Returns whether `s` is a `token`, as defined by
/// [RFC 2616](http://tools.ietf.org/html/rfc2616#page-17).
pub fn is_token(s: &[u8]) -> bool {

View file

@ -24,7 +24,7 @@ use crate::dom::bindings::inheritance::{Castable, ElementTypeId, HTMLElementType
use crate::dom::bindings::refcounted::{Trusted, TrustedPromise};
use crate::dom::bindings::reflector::DomObject;
use crate::dom::bindings::root::{Dom, DomRoot, LayoutDom, MutNullableDom, RootedReference};
use crate::dom::bindings::str::DOMString;
use crate::dom::bindings::str::{DOMString, USVString};
use crate::dom::bindings::xmlname::XMLName::InvalidXMLName;
use crate::dom::bindings::xmlname::{
namespace_from_domstring, validate_and_extract, xml_name_type,
@ -1484,20 +1484,19 @@ impl Element {
}
}
pub fn get_url_attribute(&self, local_name: &LocalName) -> DOMString {
pub fn get_url_attribute(&self, local_name: &LocalName) -> USVString {
assert!(*local_name == local_name.to_ascii_lowercase());
let attr = match self.get_attribute(&ns!(), local_name) {
Some(attr) => attr,
None => return DOMString::new(),
None => return USVString::default(),
};
let value = &**attr.value();
// XXXManishearth this doesn't handle `javascript:` urls properly
let base = document_from_node(self).base_url();
let value = base
document_from_node(self)
.base_url()
.join(value)
.map(|parsed| parsed.into_string())
.unwrap_or_else(|_| value.to_owned());
DOMString::from(value)
.map(|parsed| USVString(parsed.into_string()))
.unwrap_or_else(|_| USVString(value.to_owned()))
}
pub fn get_string_attribute(&self, local_name: &LocalName) -> DOMString {

View file

@ -12,7 +12,7 @@ use crate::dom::bindings::inheritance::Castable;
use crate::dom::bindings::refcounted::Trusted;
use crate::dom::bindings::reflector::DomObject;
use crate::dom::bindings::root::{DomRoot, LayoutDom, MutNullableDom};
use crate::dom::bindings::str::DOMString;
use crate::dom::bindings::str::{DOMString, USVString};
use crate::dom::document::Document;
use crate::dom::domtokenlist::DOMTokenList;
use crate::dom::element::{AttributeMutation, Element, RawLayoutElementHelpers};
@ -481,7 +481,7 @@ impl HTMLIFrameElementMethods for HTMLIFrameElement {
make_url_getter!(Src, "src");
// https://html.spec.whatwg.org/multipage/#dom-iframe-src
make_setter!(SetSrc, "src");
make_url_setter!(SetSrc, "src");
// https://html.spec.whatwg.org/multipage/#dom-iframe-sandbox
fn Sandbox(&self) -> DomRoot<DOMTokenList> {

View file

@ -19,7 +19,7 @@ use crate::dom::bindings::inheritance::Castable;
use crate::dom::bindings::refcounted::Trusted;
use crate::dom::bindings::reflector::DomObject;
use crate::dom::bindings::root::{DomRoot, LayoutDom, MutNullableDom};
use crate::dom::bindings::str::DOMString;
use crate::dom::bindings::str::{DOMString, USVString};
use crate::dom::document::Document;
use crate::dom::element::{reflect_cross_origin_attribute, set_cross_origin_attribute};
use crate::dom::element::{AttributeMutation, Element, RawLayoutElementHelpers};
@ -131,7 +131,7 @@ enum ImageRequestPhase {
struct ImageRequest {
state: State,
parsed_url: Option<ServoUrl>,
source_url: Option<DOMString>,
source_url: Option<USVString>,
blocker: Option<LoadBlocker>,
#[ignore_malloc_size_of = "Arc"]
image: Option<Arc<Image>>,
@ -149,7 +149,7 @@ pub struct HTMLImageElement {
generation: Cell<u32>,
#[ignore_malloc_size_of = "SourceSet"]
source_set: DomRefCell<SourceSet>,
last_selected_source: DomRefCell<Option<DOMString>>,
last_selected_source: DomRefCell<Option<USVString>>,
}
impl HTMLImageElement {
@ -456,7 +456,7 @@ impl HTMLImageElement {
fn process_image_response_for_environment_change(
&self,
image: ImageResponse,
src: DOMString,
src: USVString,
generation: u32,
selected_pixel_density: f64,
) {
@ -690,7 +690,7 @@ impl HTMLImageElement {
}
/// <https://html.spec.whatwg.org/multipage/#select-an-image-source>
fn select_image_source(&self) -> Option<(DOMString, f64)> {
fn select_image_source(&self) -> Option<(USVString, f64)> {
// Step 1, 3
self.update_source_set();
let source_set = &*self.source_set.borrow_mut();
@ -742,7 +742,7 @@ impl HTMLImageElement {
}
let selected_source = img_sources.remove(best_candidate.1).clone();
Some((
DOMString::from_string(selected_source.url),
USVString(selected_source.url),
selected_source.descriptor.den.unwrap() as f64,
))
}
@ -751,7 +751,7 @@ impl HTMLImageElement {
&self,
request: &mut RefMut<ImageRequest>,
url: &ServoUrl,
src: &DOMString,
src: &USVString,
) {
request.parsed_url = Some(url.clone());
request.source_url = Some(src.clone());
@ -763,7 +763,7 @@ impl HTMLImageElement {
}
/// Step 13-17 of html.spec.whatwg.org/multipage/#update-the-image-data
fn prepare_image_request(&self, url: &ServoUrl, src: &DOMString, selected_pixel_density: f64) {
fn prepare_image_request(&self, url: &ServoUrl, src: &USVString, selected_pixel_density: f64) {
match self.image_request.get() {
ImageRequestPhase::Pending => {
if let Some(pending_url) = self.pending_request.borrow().parsed_url.clone() {
@ -869,7 +869,7 @@ impl HTMLImageElement {
);
// Step 11
let base_url = document.base_url();
let parsed_url = base_url.join(&src);
let parsed_url = base_url.join(&src.0);
match parsed_url {
Ok(url) => {
// Step 13-17
@ -877,7 +877,7 @@ impl HTMLImageElement {
},
Err(_) => {
// Step 12.1-12.5.
let src = String::from(src);
let src = src.0;
// FIXME(nox): Why are errors silenced here?
let _ = task_source.queue(
task!(image_selected_source_error: move || {
@ -885,7 +885,7 @@ impl HTMLImageElement {
{
let mut current_request =
this.current_request.borrow_mut();
current_request.source_url = Some(src.into());
current_request.source_url = Some(USVString(src))
}
this.upcast::<EventTarget>().fire_event(atom!("error"));
this.upcast::<EventTarget>().fire_event(atom!("loadend"));
@ -907,7 +907,7 @@ impl HTMLImageElement {
let document = document_from_node(self);
let window = document.window();
let elem = self.upcast::<Element>();
let src = elem.get_string_attribute(&local_name!("src"));
let src = elem.get_url_attribute(&local_name!("src"));
let base_url = document.base_url();
// https://html.spec.whatwg.org/multipage/#reacting-to-dom-mutations
@ -929,7 +929,7 @@ impl HTMLImageElement {
// Step 3, 4
let mut selected_source = None;
let mut pixel_density = None;
let src_set = elem.get_string_attribute(&local_name!("srcset"));
let src_set = elem.get_url_attribute(&local_name!("srcset"));
let is_parent_picture = elem
.upcast::<Node>()
.GetParentElement()
@ -973,7 +973,7 @@ impl HTMLImageElement {
// Step 6.3.6
current_request.current_pixel_density = pixel_density;
let this = Trusted::new(self);
let src = String::from(src);
let src = src.0;
let _ = window.task_manager().dom_manipulation_task_source().queue(
task!(image_load_event: move || {
let this = this.root();
@ -981,7 +981,7 @@ impl HTMLImageElement {
let mut current_request =
this.current_request.borrow_mut();
current_request.parsed_url = Some(img_url);
current_request.source_url = Some(src.into());
current_request.source_url = Some(USVString(src));
}
// TODO: restart animation, if set.
this.upcast::<EventTarget>().fire_event(atom!("load"));
@ -1042,7 +1042,7 @@ impl HTMLImageElement {
// Ignore any image response for a previous request that has been discarded.
if generation == element.generation.get() {
element.process_image_response_for_environment_change(image,
DOMString::from_string(selected_source_clone), generation, selected_pixel_density);
USVString::from(selected_source_clone), generation, selected_pixel_density);
}
}),
&canceller,
@ -1088,7 +1088,7 @@ impl HTMLImageElement {
let base_url = document.base_url();
// Step 6
let img_url = match base_url.join(&selected_source) {
let img_url = match base_url.join(&selected_source.0) {
Ok(url) => url,
Err(_) => return,
};
@ -1133,7 +1133,7 @@ impl HTMLImageElement {
image_cache.clone(),
id,
self,
selected_source.to_string(),
selected_source.0,
selected_pixel_density,
);
},
@ -1152,7 +1152,7 @@ impl HTMLImageElement {
image_cache,
id,
self,
selected_source.to_string(),
selected_source.0,
selected_pixel_density,
);
self.fetch_request(&img_url, id);
@ -1163,13 +1163,13 @@ impl HTMLImageElement {
/// Step 15 for <https://html.spec.whatwg.org/multipage/#img-environment-changes>
fn finish_reacting_to_environment_change(
&self,
src: DOMString,
src: USVString,
generation: u32,
selected_pixel_density: f64,
) {
let this = Trusted::new(self);
let window = window_from_node(self);
let src = src.to_string();
let src = src.0;
let _ = window.task_manager().dom_manipulation_task_source().queue(
task!(image_load_event: move || {
let this = this.root();
@ -1180,7 +1180,7 @@ impl HTMLImageElement {
return;
}
// Step 15.2
*this.last_selected_source.borrow_mut() = Some(DOMString::from_string(src));
*this.last_selected_source.borrow_mut() = Some(USVString(src));
{
let mut pending_request = this.pending_request.borrow_mut();
@ -1463,12 +1463,12 @@ impl HTMLImageElementMethods for HTMLImageElement {
make_url_getter!(Src, "src");
// https://html.spec.whatwg.org/multipage/#dom-img-src
make_setter!(SetSrc, "src");
make_url_setter!(SetSrc, "src");
// https://html.spec.whatwg.org/multipage/#dom-img-srcset
make_getter!(Srcset, "srcset");
make_url_getter!(Srcset, "srcset");
// https://html.spec.whatwg.org/multipage/#dom-img-src
make_setter!(SetSrcset, "srcset");
make_url_setter!(SetSrcset, "srcset");
// https://html.spec.whatwg.org/multipage/#dom-img-crossOrigin
fn GetCrossOrigin(&self) -> Option<DOMString> {
@ -1560,16 +1560,16 @@ impl HTMLImageElementMethods for HTMLImageElement {
}
// https://html.spec.whatwg.org/multipage/#dom-img-currentsrc
fn CurrentSrc(&self) -> DOMString {
fn CurrentSrc(&self) -> USVString {
let current_request = self.current_request.borrow();
let ref url = current_request.parsed_url;
match *url {
Some(ref url) => DOMString::from_string(url.clone().into_string()),
Some(ref url) => USVString(url.clone().into_string()),
None => {
let ref unparsed_url = current_request.source_url;
match *unparsed_url {
Some(ref url) => url.clone(),
None => DOMString::from(""),
None => USVString("".to_owned()),
}
},
}

View file

@ -16,7 +16,7 @@ use crate::dom::bindings::error::{Error, ErrorResult};
use crate::dom::bindings::inheritance::Castable;
use crate::dom::bindings::reflector::DomObject;
use crate::dom::bindings::root::{Dom, DomRoot, LayoutDom, MutNullableDom, RootedReference};
use crate::dom::bindings::str::DOMString;
use crate::dom::bindings::str::{DOMString, USVString};
use crate::dom::compositionevent::CompositionEvent;
use crate::dom::document::Document;
use crate::dom::element::{
@ -769,7 +769,7 @@ impl HTMLInputElementMethods for HTMLInputElement {
make_url_getter!(Src, "src");
// https://html.spec.whatwg.org/multipage/#dom-input-src
make_setter!(SetSrc, "src");
make_url_setter!(SetSrc, "src");
// https://html.spec.whatwg.org/multipage/#dom-input-step
make_getter!(Step, "step");

View file

@ -9,7 +9,7 @@ use crate::dom::bindings::codegen::Bindings::HTMLLinkElementBinding;
use crate::dom::bindings::codegen::Bindings::HTMLLinkElementBinding::HTMLLinkElementMethods;
use crate::dom::bindings::inheritance::Castable;
use crate::dom::bindings::root::{DomRoot, MutNullableDom, RootedReference};
use crate::dom::bindings::str::DOMString;
use crate::dom::bindings::str::{DOMString, USVString};
use crate::dom::cssstylesheet::CSSStyleSheet;
use crate::dom::document::Document;
use crate::dom::domtokenlist::DOMTokenList;
@ -389,7 +389,7 @@ impl HTMLLinkElementMethods for HTMLLinkElement {
make_url_getter!(Href, "href");
// https://html.spec.whatwg.org/multipage/#dom-link-href
make_setter!(SetHref, "href");
make_url_setter!(SetHref, "href");
// https://html.spec.whatwg.org/multipage/#dom-link-rel
make_getter!(Rel, "rel");

View file

@ -21,7 +21,7 @@ use crate::dom::bindings::num::Finite;
use crate::dom::bindings::refcounted::Trusted;
use crate::dom::bindings::reflector::DomObject;
use crate::dom::bindings::root::{DomRoot, LayoutDom, MutNullableDom};
use crate::dom::bindings::str::DOMString;
use crate::dom::bindings::str::{DOMString, USVString};
use crate::dom::blob::Blob;
use crate::dom::document::Document;
use crate::dom::element::{AttributeMutation, Element};
@ -1279,7 +1279,7 @@ impl HTMLMediaElementMethods for HTMLMediaElement {
make_url_getter!(Src, "src");
// https://html.spec.whatwg.org/multipage/#dom-media-src
make_setter!(SetSrc, "src");
make_url_setter!(SetSrc, "src");
// https://html.spec.whatwg.org/multipage/#dom-media-srcobject
fn GetSrcObject(&self) -> Option<DomRoot<Blob>> {
@ -1299,8 +1299,8 @@ impl HTMLMediaElementMethods for HTMLMediaElement {
make_setter!(SetPreload, "preload");
// https://html.spec.whatwg.org/multipage/#dom-media-currentsrc
fn CurrentSrc(&self) -> DOMString {
DOMString::from(self.current_src.borrow().clone())
fn CurrentSrc(&self) -> USVString {
USVString(self.current_src.borrow().clone())
}
// https://html.spec.whatwg.org/multipage/#dom-media-load

View file

@ -12,7 +12,7 @@ use crate::dom::bindings::inheritance::Castable;
use crate::dom::bindings::refcounted::Trusted;
use crate::dom::bindings::reflector::DomObject;
use crate::dom::bindings::root::{Dom, DomRoot, RootedReference};
use crate::dom::bindings::str::DOMString;
use crate::dom::bindings::str::{DOMString, USVString};
use crate::dom::document::Document;
use crate::dom::element::{
cors_setting_for_element, reflect_cross_origin_attribute, set_cross_origin_attribute,
@ -808,7 +808,7 @@ impl HTMLScriptElementMethods for HTMLScriptElement {
make_url_getter!(Src, "src");
// https://html.spec.whatwg.org/multipage/#dom-script-src
make_setter!(SetSrc, "src");
make_url_setter!(SetSrc, "src");
// https://html.spec.whatwg.org/multipage/#dom-script-type
make_getter!(Type, "type");

View file

@ -96,7 +96,7 @@ macro_rules! make_uint_getter(
#[macro_export]
macro_rules! make_url_getter(
( $attr:ident, $htmlname:tt ) => (
fn $attr(&self) -> DOMString {
fn $attr(&self) -> USVString {
use crate::dom::bindings::inheritance::Castable;
use crate::dom::element::Element;
let element = self.upcast::<Element>();
@ -105,6 +105,19 @@ macro_rules! make_url_getter(
);
);
#[macro_export]
macro_rules! make_url_setter(
( $attr:ident, $htmlname:tt ) => (
fn $attr(&self, value: USVString) {
use crate::dom::bindings::inheritance::Castable;
use crate::dom::element::Element;
let element = self.upcast::<Element>();
element.set_string_attribute(&local_name!($htmlname),
DOMString::from(value.0));
}
);
);
#[macro_export]
macro_rules! make_form_action_getter(
( $attr:ident, $htmlname:tt ) => (
@ -171,18 +184,6 @@ macro_rules! make_bool_setter(
);
);
#[macro_export]
macro_rules! make_url_setter(
( $attr:ident, $htmlname:tt ) => (
fn $attr(&self, value: DOMString) {
use dom::bindings::inheritance::Castable;
use dom::element::Element;
let element = self.upcast::<Element>();
element.set_url_attribute(&local_name!($htmlname), value);
}
);
);
#[macro_export]
macro_rules! make_uint_setter(
($attr:ident, $htmlname:tt, $default:expr) => (

View file

@ -16,7 +16,7 @@ use crate::dom::bindings::refcounted::Trusted;
use crate::dom::bindings::reflector::{reflect_dom_object, DomObject, Reflector};
use crate::dom::bindings::root::{Dom, DomRoot, MutNullableDom, RootedReference};
use crate::dom::bindings::settings_stack::is_execution_stack_empty;
use crate::dom::bindings::str::DOMString;
use crate::dom::bindings::str::{DOMString, USVString};
use crate::dom::characterdata::CharacterData;
use crate::dom::comment::Comment;
use crate::dom::document::{Document, DocumentSource, HasBrowsingContext, IsHTMLDocument};
@ -722,7 +722,7 @@ impl FetchResponseListener for ParserContext {
let doc = &parser.document;
let doc_body = DomRoot::upcast::<Node>(doc.GetBody().unwrap());
let img = HTMLImageElement::new(local_name!("img"), None, doc);
img.SetSrc(DOMString::from(self.url.to_string()));
img.SetSrc(USVString(self.url.to_string()));
doc_body
.AppendChild(&DomRoot::upcast::<Node>(img))
.expect("Appending failed");

View file

@ -6,7 +6,7 @@
[HTMLConstructor]
interface HTMLIFrameElement : HTMLElement {
[CEReactions]
attribute DOMString src;
attribute USVString src;
// [CEReactions]
// attribute DOMString srcdoc;

View file

@ -8,9 +8,9 @@ interface HTMLImageElement : HTMLElement {
[CEReactions]
attribute DOMString alt;
[CEReactions]
attribute DOMString src;
attribute USVString src;
[CEReactions]
attribute DOMString srcset;
attribute USVString srcset;
[CEReactions]
attribute DOMString? crossOrigin;
[CEReactions]
@ -24,7 +24,7 @@ interface HTMLImageElement : HTMLElement {
readonly attribute unsigned long naturalWidth;
readonly attribute unsigned long naturalHeight;
readonly attribute boolean complete;
readonly attribute DOMString currentSrc;
readonly attribute USVString currentSrc;
// also has obsolete members
};

View file

@ -61,7 +61,7 @@ interface HTMLInputElement : HTMLElement {
[CEReactions, SetterThrows]
attribute unsigned long size;
[CEReactions]
attribute DOMString src;
attribute USVString src;
[CEReactions]
attribute DOMString step;
[CEReactions]

View file

@ -6,7 +6,7 @@
[HTMLConstructor]
interface HTMLLinkElement : HTMLElement {
[CEReactions]
attribute DOMString href;
attribute USVString href;
[CEReactions]
attribute DOMString? crossOrigin;
[CEReactions]

View file

@ -13,9 +13,9 @@ interface HTMLMediaElement : HTMLElement {
readonly attribute MediaError? error;
// network state
[CEReactions] attribute DOMString src;
[CEReactions] attribute USVString src;
attribute MediaProvider? srcObject;
readonly attribute DOMString currentSrc;
readonly attribute USVString currentSrc;
// [CEReactions] attribute DOMString crossOrigin;
const unsigned short NETWORK_EMPTY = 0;
const unsigned short NETWORK_IDLE = 1;

View file

@ -6,7 +6,7 @@
[HTMLConstructor]
interface HTMLScriptElement : HTMLElement {
[CEReactions]
attribute DOMString src;
attribute USVString src;
[CEReactions]
attribute DOMString type;
[CEReactions]