Skip to content

Commit

Permalink
Expand InputType to cover all possible types
Browse files Browse the repository at this point in the history
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
1 parent 005fd9c commit 05bfb8d
Show file tree
Hide file tree
Showing 6 changed files with 234 additions and 140 deletions.
1 change: 1 addition & 0 deletions components/atoms/static_atoms.txt
Expand Up @@ -55,6 +55,7 @@ playing
print
progress
radio
range
readystatechange
reftest-wait
reset
Expand Down
4 changes: 2 additions & 2 deletions components/script/dom/htmlelement.rs
Expand Up @@ -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};
Expand Down Expand Up @@ -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 |
Expand Down
4 changes: 2 additions & 2 deletions components/script/dom/htmlformelement.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit 05bfb8d

Please sign in to comment.