Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement URL.searchParams #10351

Merged
merged 1 commit into from Apr 6, 2016
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -6,11 +6,13 @@ use dom::bindings::cell::DOMRefCell;
use dom::bindings::codegen::Bindings::URLBinding::{self, URLMethods};
use dom::bindings::error::{Error, ErrorResult, Fallible};
use dom::bindings::global::GlobalRef;
use dom::bindings::js::Root;
use dom::bindings::reflector::{Reflector, reflect_dom_object};
use dom::bindings::js::{JS, MutNullableHeap, Root};
use dom::bindings::reflector::{Reflectable, Reflector, reflect_dom_object};
use dom::bindings::str::USVString;
use dom::urlhelper::UrlHelper;
use dom::urlsearchparams::URLSearchParams;
use std::borrow::ToOwned;
use std::default::Default;
use url::{Host, ParseResult, Url, UrlParser};
use util::str::DOMString;

@@ -24,6 +26,9 @@ pub struct URL {

// https://url.spec.whatwg.org/#concept-urlutils-get-the-base
base: Option<Url>,

// https://url.spec.whatwg.org/#dom-url-searchparams
search_params: MutNullableHeap<JS<URLSearchParams>>,
}

impl URL {
@@ -32,13 +37,18 @@ impl URL {
reflector_: Reflector::new(),
url: DOMRefCell::new(url),
base: base,
search_params: Default::default(),
}
}

pub fn new(global: GlobalRef, url: Url, base: Option<Url>) -> Root<URL> {
reflect_dom_object(box URL::new_inherited(url, base),
global, URLBinding::Wrap)
}

pub fn set_query(&self, query: String) {
self.url.borrow_mut().query = Some(query);
}
}

impl URL {
@@ -69,8 +79,13 @@ impl URL {
return Err(Error::Type(format!("could not parse URL: {}", error)));
}
};
// Steps 5-8.
Ok(URL::new(global, parsed_url, parsed_base))
// Step 5: Skip (see step 8 below).
// Steps 6-7.
let result = URL::new(global, parsed_url, parsed_base);
// Step 8: Instead of construcing a new `URLSearchParams` object here, construct it
// on-demand inside `URL::SearchParams`.
// Step 9.
Ok(result)
}

// https://url.spec.whatwg.org/#dom-url-domaintoasciidomain
@@ -189,6 +204,14 @@ impl URLMethods for URL {
// https://url.spec.whatwg.org/#dom-url-search
fn SetSearch(&self, value: USVString) {
UrlHelper::SetSearch(&mut self.url.borrow_mut(), value);

This comment has been minimized.

@Manishearth

Manishearth Apr 2, 2016

Member

All the setters should update the stored query object if we're going to store it as a field.

This comment has been minimized.

@stjepang

stjepang Apr 2, 2016

Author Contributor

Instead of calculating query pairs on-demand, I'd rather stick to the spec and store them immediately.

Regarding iteration, this is something we currently don't handle - it's commented in URLSearchParams.webidl:
// iterable<USVString, USVString>;
Please correct me if I'm wrong.

Firefox's approach looks reasonable to me. We'll just have to be careful when implementing the iterable interface.

if let Some(search_params) = self.search_params.get() {
search_params.set_list(self.url.borrow().query_pairs().unwrap_or_else(|| vec![]));
}
}

// https://url.spec.whatwg.org/#dom-url-searchparams
fn SearchParams(&self) -> Root<URLSearchParams> {
self.search_params.or_init(|| URLSearchParams::new(self.global().r(), Some(self)))
}

// https://url.spec.whatwg.org/#dom-url-href
@@ -11,6 +11,8 @@ use dom::bindings::global::GlobalRef;
use dom::bindings::js::Root;
use dom::bindings::reflector::{Reflector, reflect_dom_object};
use dom::bindings::str::USVString;
use dom::bindings::weakref::MutableWeakRef;
use dom::url::URL;
use encoding::types::EncodingRef;
use url::form_urlencoded::{parse, serialize_with_encoding};
use util::str::DOMString;
@@ -21,26 +23,29 @@ pub struct URLSearchParams {
reflector_: Reflector,
// https://url.spec.whatwg.org/#concept-urlsearchparams-list
list: DOMRefCell<Vec<(String, String)>>,
// https://url.spec.whatwg.org/#concept-urlsearchparams-url-object
url: MutableWeakRef<URL>,
}

impl URLSearchParams {
fn new_inherited() -> URLSearchParams {
fn new_inherited(url: Option<&URL>) -> URLSearchParams {
URLSearchParams {
reflector_: Reflector::new(),
list: DOMRefCell::new(vec![]),
url: MutableWeakRef::new(url),
}
}

pub fn new(global: GlobalRef) -> Root<URLSearchParams> {
reflect_dom_object(box URLSearchParams::new_inherited(), global,
pub fn new(global: GlobalRef, url: Option<&URL>) -> Root<URLSearchParams> {
reflect_dom_object(box URLSearchParams::new_inherited(url), global,
URLSearchParamsBinding::Wrap)
}

// https://url.spec.whatwg.org/#dom-urlsearchparams-urlsearchparams
pub fn Constructor(global: GlobalRef, init: Option<USVStringOrURLSearchParams>) ->
Fallible<Root<URLSearchParams>> {
// Step 1.
let query = URLSearchParams::new(global);
let query = URLSearchParams::new(global, None);
match init {
Some(USVStringOrURLSearchParams::USVString(init)) => {
// Step 2.
@@ -55,6 +60,10 @@ impl URLSearchParams {
// Step 4.
Ok(query)
}

pub fn set_list(&self, list: Vec<(String, String)>) {
*self.list.borrow_mut() = list;
}
}

impl URLSearchParamsMethods for URLSearchParams {
@@ -101,6 +110,7 @@ impl URLSearchParamsMethods for URLSearchParams {

// https://url.spec.whatwg.org/#dom-urlsearchparams-set
fn Set(&self, name: USVString, value: USVString) {
// Step 1.
let mut list = self.list.borrow_mut();
let mut index = None;
let mut i = 0;
@@ -118,8 +128,9 @@ impl URLSearchParamsMethods for URLSearchParams {
});
match index {
Some(index) => list[index].1 = value.0,
None => list.push((name.0, value.0)),
None => list.push((name.0, value.0)), // Step 2.
};
// Step 3.
self.update_steps();
}

@@ -140,8 +151,10 @@ impl URLSearchParams {


impl URLSearchParams {
// https://url.spec.whatwg.org/#concept-uq-update
// https://url.spec.whatwg.org/#concept-urlsearchparams-update
fn update_steps(&self) {
// XXXManishearth Implement this when the URL interface is implemented
if let Some(url) = self.url.root() {
url.set_query(self.serialize(None));
}
}
}
@@ -20,7 +20,7 @@ interface URL {
attribute USVString port;
attribute USVString pathname;
attribute USVString search;
// readonly attribute URLSearchParams searchParams;
readonly attribute URLSearchParams searchParams;
attribute USVString hash;

// This is only doing as well as gecko right now.
@@ -15,6 +15,8 @@ interface URLSearchParams {
sequence<USVString> getAll(USVString name);
boolean has(USVString name);
void set(USVString name, USVString value);
// Be careful with implementing iterable interface.
// Search params might be mutated by URL::SetSearch while iterating (discussed in PR #10351).
// iterable<USVString, USVString>;
stringifier;
};
@@ -3,12 +3,6 @@
[URL interface: operation domainToUnicode(ScalarValueString)]
expected: FAIL

[URL interface: attribute searchParams]
expected: FAIL

[URL interface: new URL("http://foo") must inherit property "searchParams" with the proper type (12)]
expected: FAIL

[URL interface: operation domainToUnicode(USVString)]
expected: FAIL

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.