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

Subsequent step for implementing HTML form validation #14566

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -455,7 +455,27 @@ impl HTMLFormElement {
// Step 1-3
let _unhandled_invalid_controls = match self.static_validation() {
Ok(()) => return Ok(()),
Err(err) => err
//Err(err) => err,
Err(err) => {
match err[0] {
FormSubmittableElement::InputElement(ref i) =>
println!("First element didn't pass: {}",

This comment has been minimized.

Copy link
@Manishearth

Manishearth Dec 13, 2016

Member

Please don't print to stdout.

@jdm Thought on how this should be reported?

This comment has been minimized.

Copy link
@jdm

jdm Dec 15, 2016

Member

Stdout is all we've got at the moment. We should be calling Focus() on the element, however.

i.upcast::<Element>().get_string_attribute(&local_name!("name"))),
FormSubmittableElement::ButtonElement(ref i) =>
println!("First element didn't pass: {}",
i.upcast::<Element>().get_string_attribute(&local_name!("name"))),
FormSubmittableElement::SelectElement(ref i) =>
println!("First element didn't pass: {}",
i.upcast::<Element>().get_string_attribute(&local_name!("name"))),
FormSubmittableElement::TextAreaElement(ref i) =>
println!("First elment didn't pass: {}",
i.upcast::<Element>().get_string_attribute(&local_name!("name"))),
FormSubmittableElement::ObjectElement(ref i) =>
println!("First element didn't pass: {}",
i.upcast::<Element>().get_string_attribute(&local_name!("name"))),

This comment has been minimized.

Copy link
@jdm

jdm Dec 15, 2016

Member

As discussed in the evaluation, we should find a way to reduce code duplication here. The reporting belongs in the code beneath here, where it should replace the TODO.

};
}

};
// TODO: Report the problems with the constraints of at least one of
// the elements given in unhandled invalid controls to the user
@@ -11,6 +11,7 @@ use dom::bindings::codegen::Bindings::FileListBinding::FileListMethods;
use dom::bindings::codegen::Bindings::HTMLInputElementBinding;
use dom::bindings::codegen::Bindings::HTMLInputElementBinding::HTMLInputElementMethods;
use dom::bindings::codegen::Bindings::KeyboardEventBinding::KeyboardEventMethods;
use dom::bindings::codegen::Bindings::ValidityStateBinding::ValidityStateMethods;
use dom::bindings::error::{Error, ErrorResult};
use dom::bindings::inheritance::Castable;
use dom::bindings::js::{JS, LayoutJS, MutNullableJS, Root, RootedReference};
@@ -32,6 +33,7 @@ use dom::node::{document_from_node, window_from_node};
use dom::nodelist::NodeList;
use dom::validation::Validatable;
use dom::validitystate::ValidationFlags;
use dom::validitystate::ValidityState;
use dom::virtualmethods::VirtualMethods;
use html5ever_atoms::LocalName;
use ipc_channel::ipc::{self, IpcSender};
@@ -1139,6 +1141,21 @@ impl Validatable for HTMLInputElement {
}
fn validate(&self, _validate_flags: ValidationFlags) -> bool {
// call stub methods defined in validityState.rs file here according to the flags set in validate_flags
let window = window_from_node(self);
let el = self.upcast::<Element>();
let vs = ValidityState::new(&window, el);
if vs.ValueMissing() {

This comment has been minimized.

Copy link
@jdm

jdm Dec 15, 2016

Member

I know the comment above says to call methods in validityState, and I apologize for missing that in the previous group's work. However, the model in this PR is backwards - validation code for a particular element type belongs in this validate method, and the methods in ValidityState should be calling this method and passing appropriate ValidationFlags. This method should be checking the state of validate_flags and performing the appropriate validation checks that are requested.

return false;
}
if vs.TooLong() {
return false;
}
if vs.TooShort() {
return false;
}
if vs.TypeMismatch() {
return false;
}
true
}
}
@@ -8,6 +8,8 @@ use dom::bindings::js::{JS, Root};
use dom::bindings::reflector::{Reflector, reflect_dom_object};
use dom::element::Element;
use dom::window::Window;
use num_traits::ToPrimitive;
use regex::Regex;

// https://html.spec.whatwg.org/multipage/#validity-states
#[derive(JSTraceable)]
@@ -69,11 +71,36 @@ impl ValidityState {
impl ValidityStateMethods for ValidityState {
// https://html.spec.whatwg.org/multipage/#dom-validitystate-valuemissing
fn ValueMissing(&self) -> bool {
if self.element.has_attribute(&local_name!("required")) {
let v = self.element.get_string_attribute(&local_name!("value"));
let n = self.element.get_string_attribute(&local_name!("name"));
if v.is_empty() {
println!("{} Value missing!", n);
return true;
}
}
false
}

// https://html.spec.whatwg.org/multipage/#dom-validitystate-typemismatch
fn TypeMismatch(&self) -> bool {
if self.element.has_attribute(&local_name!("type")) {
let content_type = self.element.get_string_attribute(&local_name!("type"));
let content_value = self.element.get_string_attribute(&local_name!("value"));
let re = match content_type.to_string().as_ref() {
"email" => Regex::new(r"^\w+@[a-zA-Z_]+?\.[a-zA-Z]{2,3}$").unwrap(),
"url" => Regex::new(r"^[a-zA-Z0-9\-\.]+\.(com|org|net|mil|edu|COM|ORG|NET|MIL|EDU)$").unwrap(),
"date" => Regex::new(r"^\d{1,2}\/\d{1,2}\/\d{4}$").unwrap(),
"tel" => Regex::new(r"^\d{3}-\d{2}-\d{4}$").unwrap(),

This comment has been minimized.

Copy link
@jdm

jdm Dec 15, 2016

Member

Unfortunately these regexes need to be either much more complex, or much less strict. I lean towards much less strict - URLs can check if there's a period present; emails can check if there's an @; telephone numbers should not contain letters.

_ => Regex::new(r".*?").unwrap()
};
if re.is_match(&(content_value.to_string())) {
println!("Syntax wrong for {}", content_value);
return false;
} else {
return true;
}
}
false
}

@@ -84,11 +111,33 @@ impl ValidityStateMethods for ValidityState {

// https://html.spec.whatwg.org/multipage/#dom-validitystate-toolong
fn TooLong(&self) -> bool {
if self.element.has_attribute(&local_name!("maxlength")) {
let Max = self.element.get_string_attribute(&local_name!("maxlength")).to_string();
let v = self.element.get_string_attribute(&local_name!("value")).to_string();
let n = self.element.get_string_attribute(&local_name!("name"));
let l = v.len().to_i32().unwrap();
if l > Max.parse::<i32>().unwrap() {
println!("{} is too long for {}, maxlength is {} characters.", v, n, Max);
return true;
}

This comment has been minimized.

Copy link
@jdm

jdm Dec 15, 2016

Member

HTMLInputElement already contains the maxlength information in a field, so this check will be more straightforward in HTMLInputElement::validate.

}

false
}

// https://html.spec.whatwg.org/multipage/#dom-validitystate-tooshort
fn TooShort(&self) -> bool {
if self.element.has_attribute(&local_name!("minlength")) {
let Min = self.element.get_string_attribute(&local_name!("minlength")).to_string();
let v = self.element.get_string_attribute(&local_name!("value")).to_string();
let n = self.element.get_string_attribute(&local_name!("name"));
let l = v.len().to_i32().unwrap();
if l < Min.parse::<i32>().unwrap() {
println!("{} is too short for {}, minlength is {} characters.", v, n, Min);
return true;
}
}

false
}

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