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

Expand InputType to cover all possible types #19471

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

Always

Just for now

Expand InputType to cover all possible types

This came out of a conversation with nox in IRC:
https://mozilla.logbot.info/servo/20171201#c13946454-c13946594

The code I was working on which motivated this change is here:
#19461

Previously, InputType::Text was used to represent several different
values of the type attribute on an input element.

If an input element doesn't have a type attribute, or its type attribute
doesn't contain a recognised value, then the input's type defaults to
"text".

Before this change, there were a number of checks in the code which
directly looked at the type attribute. If those checks matched against
the value "text", then they were potentially buggy, since an input with
type=invalid should also behave like an input with type=text.

Rather than have every conditional which cares about the input type also
have to deal with invalid input types, we can convert the type attribute
to an InputType enum once, and then match against the enum.

A secondary benefit is that the compiler can tell us whether we've
missed branches in a match expression. While working on this I
discovered that the HTMLInputElement::value_mode() method misses a case
for inputs with type=hidden (this resulted in a failing WPT test
passing).

I've also implemented the Default trait for InputType, so we now only
have one place in the code which knows that InputType::Text is the
default, where previously there were several.
  • Loading branch information
jonleighton committed Dec 6, 2017
commit 05bfb8d07aec90a232d12e503dc611d3dbd4bc0a
@@ -55,6 +55,7 @@ playing
print
progress
radio
range
readystatechange
reftest-wait
reset
@@ -22,7 +22,7 @@ use dom::eventtarget::EventTarget;
use dom::htmlbodyelement::HTMLBodyElement;
use dom::htmlframesetelement::HTMLFrameSetElement;
use dom::htmlhtmlelement::HTMLHtmlElement;
use dom::htmlinputelement::HTMLInputElement;
use dom::htmlinputelement::{HTMLInputElement, InputType};
use dom::htmllabelelement::HTMLLabelElement;
use dom::node::{Node, NodeFlags};
use dom::node::{document_from_node, window_from_node};
@@ -497,7 +497,7 @@ impl HTMLElement {
NodeTypeId::Element(ElementTypeId::HTMLElement(type_id)) =>
match type_id {
HTMLElementTypeId::HTMLInputElement =>
self.downcast::<HTMLInputElement>().unwrap().type_() != atom!("hidden"),
self.downcast::<HTMLInputElement>().unwrap().input_type() != InputType::Hidden,
HTMLElementTypeId::HTMLButtonElement |
HTMLElementTypeId::HTMLMeterElement |
HTMLElementTypeId::HTMLOutputElement |
@@ -30,7 +30,7 @@ use dom::htmlelement::HTMLElement;
use dom::htmlfieldsetelement::HTMLFieldSetElement;
use dom::htmlformcontrolscollection::HTMLFormControlsCollection;
use dom::htmlimageelement::HTMLImageElement;
use dom::htmlinputelement::HTMLInputElement;
use dom::htmlinputelement::{HTMLInputElement, InputType};
use dom::htmllabelelement::HTMLLabelElement;
use dom::htmllegendelement::HTMLLegendElement;
use dom::htmlobjectelement::HTMLObjectElement;
@@ -183,7 +183,7 @@ impl HTMLFormElementMethods for HTMLFormElement {
}
HTMLElementTypeId::HTMLInputElement => {
let input_elem = elem.downcast::<HTMLInputElement>().unwrap();
if input_elem.type_() == atom!("image") {
if input_elem.input_type() == InputType::Image {
return false;
}
input_elem.form_owner()
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.