Skip to content

Commit

Permalink
Improved: Use browser validation for input element, support types
Browse files Browse the repository at this point in the history
Rely on browser native validation for inputs, using the "required"
attribute, instead of jQuery Validation plugin.
Support "number", "email", "url" and "tel" input types.
Support "pattern" attribute for input types that supports it.
Fix boolean attributes uses in FTL macros ("readonly", "required").
Fix required behavior when "required-field-style" is not empty.
Add "required" attribute on password fields.
Add "required" attribute to inputs in login forms

OFBIZ-13183
  • Loading branch information
florianMo committed Dec 5, 2024
1 parent 531f2bd commit 488d1a0
Show file tree
Hide file tree
Showing 9 changed files with 239 additions and 29 deletions.
18 changes: 18 additions & 0 deletions framework/widget/dtd/widget-form.xsd
Original file line number Diff line number Diff line change
Expand Up @@ -1497,6 +1497,24 @@ under the License.
<xs:documentation>Specifies a short hint that describes the expected value of an input field.</xs:documentation>
</xs:annotation>
</xs:attribute>
<xs:attribute name="type" type="xs:string">
<xs:annotation>
<xs:documentation>Controls the type attribute on the input element. Limited to a few types. See https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input#input_types</xs:documentation>
</xs:annotation>
<xs:simpleType>
<xs:restriction base="xs:token">
<xs:enumeration value="number" />
<xs:enumeration value="email" />
<xs:enumeration value="url" />
<xs:enumeration value="tel" />
</xs:restriction>
</xs:simpleType>
</xs:attribute>
<xs:attribute name="pattern" type="xs:string">
<xs:annotation>
<xs:documentation>Controls the pattern attribute of the input element. See https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/pattern</xs:documentation>
</xs:annotation>
</xs:attribute>
</xs:complexType>
</xs:element>
<xs:element name="textarea" substitutionGroup="AllFields">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5441,12 +5441,16 @@ public static class TextField extends FieldInfo {
private final boolean readonly;
private final int size;
private final SubHyperlink subHyperlink;
private final String type;
private final String pattern;

public TextField(Element element, ModelFormField modelFormField) {
super(element, modelFormField);
this.clientAutocompleteField = !"false".equals(element.getAttribute("client-autocomplete-field"));
this.defaultValue = FlexibleStringExpander.getInstance(element.getAttribute("default-value"));
this.mask = element.getAttribute("mask");
this.type = element.getAttribute("type");
this.pattern = element.getAttribute("pattern");
Integer maxlength = null;
String maxlengthStr = element.getAttribute("maxlength");
if (!maxlengthStr.isEmpty()) {
Expand Down Expand Up @@ -5484,6 +5488,8 @@ protected TextField(int fieldSource, int size, Integer maxlength, ModelFormField
this.clientAutocompleteField = true;
this.defaultValue = FlexibleStringExpander.getInstance("");
this.mask = "";
this.type = "";
this.pattern = "";
this.maxlength = maxlength;
this.placeholder = FlexibleStringExpander.getInstance("");
this.readonly = false;
Expand All @@ -5496,6 +5502,8 @@ protected TextField(int fieldSource, int size, Integer maxlength, ModelFormField
this.clientAutocompleteField = true;
this.defaultValue = FlexibleStringExpander.getInstance("");
this.mask = "";
this.type = "";
this.pattern = "";
this.maxlength = maxlength;
this.placeholder = FlexibleStringExpander.getInstance("");
this.readonly = false;
Expand All @@ -5508,6 +5516,8 @@ private TextField(int fieldSource, int fieldType, ModelFormField modelFormField)
this.clientAutocompleteField = true;
this.defaultValue = FlexibleStringExpander.getInstance("");
this.mask = "";
this.type = "";
this.pattern = "";
this.maxlength = null;
this.placeholder = FlexibleStringExpander.getInstance("");
this.readonly = false;
Expand All @@ -5528,6 +5538,8 @@ protected TextField(TextField original, ModelFormField modelFormField) {
this.clientAutocompleteField = original.clientAutocompleteField;
this.defaultValue = original.defaultValue;
this.mask = original.mask;
this.type = original.type;
this.pattern = original.pattern;
this.placeholder = original.placeholder;
this.size = original.size;
this.maxlength = original.maxlength;
Expand Down Expand Up @@ -5639,6 +5651,14 @@ public void renderFieldString(Appendable writer, Map<String, Object> context, Fo
throws IOException {
formStringRenderer.renderTextField(writer, context, this);
}

public String getType() {
return this.type;
}

public String getPattern() {
return this.pattern;
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,12 +253,20 @@ public RenderableFtl textField(final Map<String, Object> context, final ModelFor
final boolean javaScriptEnabled) {
ModelFormField modelFormField = textField.getModelFormField();
String name = modelFormField.getParameterName(context);
String className = "";
String type = textField.getType();
if (UtilValidate.isEmpty(type)) {
type = "text";
}
String pattern = "";
if (List.of("text", "email", "url", "tel").contains(type)) {
pattern = textField.getPattern();
}
List<String> classes = new ArrayList<>();
String alert = "false";
String mask = "";
String placeholder = textField.getPlaceholder(context);
if (UtilValidate.isNotEmpty(modelFormField.getWidgetStyle())) {
className = modelFormField.getWidgetStyle();
classes.add(modelFormField.getWidgetStyle());
if (modelFormField.shouldBeRed(context)) {
alert = "true";
}
Expand All @@ -275,14 +283,13 @@ public RenderableFtl textField(final Map<String, Object> context, final ModelFor
String clientAutocomplete = "false";
//check for required field style on single forms
if ("single".equals(modelFormField.getModelForm().getType()) && modelFormField.getRequiredField()) {
// kept for backward compatibility with existing CSS/JS
// maybe unused if jQuery Validation is no longer used
// for styling we should rely on "required" attribute
classes.add("required");
String requiredStyle = modelFormField.getRequiredFieldStyle();
if (UtilValidate.isEmpty(requiredStyle)) {
requiredStyle = "required";
}
if (UtilValidate.isEmpty(className)) {
className = requiredStyle;
} else {
className = requiredStyle + " " + className;
if (UtilValidate.isNotEmpty(requiredStyle)) {
classes.add(requiredStyle);
}
}
List<ModelForm.UpdateArea> updateAreas = modelFormField.getOnChangeUpdateAreas();
Expand All @@ -301,7 +308,9 @@ public RenderableFtl textField(final Map<String, Object> context, final ModelFor
return RenderableFtlMacroCall.builder()
.name("renderTextField")
.stringParameter("name", name)
.stringParameter("className", className)
.stringParameter("className", String.join(" ", classes))
.stringParameter("type", type)
.stringParameter("pattern", pattern)
.stringParameter("alert", alert)
.stringParameter("value", value)
.stringParameter("textSize", textSize)
Expand All @@ -311,6 +320,7 @@ public RenderableFtl textField(final Map<String, Object> context, final ModelFor
.stringParameter("action", action != null ? action : "")
.booleanParameter("disabled", disabled)
.booleanParameter("readonly", readonly)
.booleanParameter("required", modelFormField.getRequiredField())
.stringParameter("clientAutocomplete", clientAutocomplete)
.stringParameter("ajaxUrl", ajaxUrl)
.booleanParameter("ajaxEnabled", ajaxEnabled)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.apache.ofbiz.widget.renderer.macro.renderable.RenderableFtl;
import org.apache.ofbiz.widget.renderer.macro.renderable.RenderableFtlNoop;
import org.junit.Test;
import org.mockito.Mockito;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
Expand All @@ -46,6 +47,8 @@

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;

public class RenderableFtlFormElementsBuilderTest {

Expand Down Expand Up @@ -240,6 +243,153 @@ public void textFieldCreatesAjaxUrl(@Mocked final ModelFormField.TextField textF
+ "areaId2,http://host.domain/target2,")));
}

@Test
public void textFieldDefaultTypeIsText(@Mocked final ModelFormField.TextField textField) {
new Expectations() {
{
httpSession.getAttribute("delegatorName"); result = "DelegatorName";
}
};

final RenderableFtl renderableFtl = renderableFtlFormElementsBuilder.textField(Map.of("session", httpSession), textField, true);
assertThat(renderableFtl, MacroCallMatcher.hasName("renderTextField"));
assertThat(renderableFtl, MacroCallMatcher.hasParameters(MacroCallParameterMatcher.hasNameAndStringValue("type", "text")));
}

@Test
public void textFieldTypeNumber(@Mocked final ModelFormField.TextField textField) {
new Expectations() {
{
textField.getType(); result = "number";
httpSession.getAttribute("delegatorName"); result = "DelegatorName";
}
};

final RenderableFtl renderableFtl = renderableFtlFormElementsBuilder.textField(Map.of("session", httpSession), textField, true);
assertThat(renderableFtl, MacroCallMatcher.hasName("renderTextField"));
assertThat(renderableFtl, MacroCallMatcher.hasParameters(MacroCallParameterMatcher.hasNameAndStringValue("type", "number")));
}

@Test
public void textFieldTypeEmail(@Mocked final ModelFormField.TextField textField) {
new Expectations() {
{
textField.getType(); result = "email";
httpSession.getAttribute("delegatorName"); result = "DelegatorName";
}
};

final RenderableFtl renderableFtl = renderableFtlFormElementsBuilder.textField(Map.of("session", httpSession), textField, true);
assertThat(renderableFtl, MacroCallMatcher.hasName("renderTextField"));
assertThat(renderableFtl, MacroCallMatcher.hasParameters(MacroCallParameterMatcher.hasNameAndStringValue("type", "email")));
}

@Test
public void textFieldTypeUrl(@Mocked final ModelFormField.TextField textField) {
new Expectations() {
{
textField.getType(); result = "url";
httpSession.getAttribute("delegatorName"); result = "DelegatorName";
}
};

final RenderableFtl renderableFtl = renderableFtlFormElementsBuilder.textField(Map.of("session", httpSession), textField, true);
assertThat(renderableFtl, MacroCallMatcher.hasName("renderTextField"));
assertThat(renderableFtl, MacroCallMatcher.hasParameters(MacroCallParameterMatcher.hasNameAndStringValue("type", "url")));
}

@Test
public void textFieldTypeTel(@Mocked final ModelFormField.TextField textField) {
new Expectations() {
{
textField.getType(); result = "tel";
httpSession.getAttribute("delegatorName"); result = "DelegatorName";
}
};

final RenderableFtl renderableFtl = renderableFtlFormElementsBuilder.textField(Map.of("session", httpSession), textField, true);
assertThat(renderableFtl, MacroCallMatcher.hasName("renderTextField"));
assertThat(renderableFtl, MacroCallMatcher.hasParameters(MacroCallParameterMatcher.hasNameAndStringValue("type", "tel")));
}

@Test
public void textFieldNotRequired(@Mocked final ModelFormField.TextField textField) {
new Expectations() {
{
modelFormField.getRequiredField(); result = false;
httpSession.getAttribute("delegatorName"); result = "DelegatorName";
}
};

final RenderableFtl renderableFtl = renderableFtlFormElementsBuilder.textField(Map.of("session", httpSession), textField, true);
assertThat(renderableFtl, MacroCallMatcher.hasName("renderTextField"));
assertThat(renderableFtl, MacroCallMatcher.hasParameters(MacroCallParameterMatcher.hasNameAndBooleanValue("required", false)));
}

@Test
public void textFieldRequiredWithoutRequiredStyle(@Mocked final ModelFormField.TextField textField) {
new Expectations() {
{
modelFormField.getModelForm().getType(); result = "single";
modelFormField.getRequiredField(); result = true;
httpSession.getAttribute("delegatorName"); result = "DelegatorName";
}
};

final RenderableFtl renderableFtl = renderableFtlFormElementsBuilder.textField(Map.of("session", httpSession), textField, true);
assertThat(renderableFtl, MacroCallMatcher.hasName("renderTextField"));
assertThat(renderableFtl, MacroCallMatcher.hasParameters(MacroCallParameterMatcher.hasNameAndBooleanValue("required", true)));
assertThat(renderableFtl, MacroCallMatcher.hasParameters(MacroCallParameterMatcher.hasNameAndStringValue("className", "required")));
}

@Test
public void textFieldRequiredWithRequiredStyle(@Mocked final ModelFormField.TextField textField) {
new Expectations() {
{
modelFormField.getModelForm().getType(); result = "single";
modelFormField.getRequiredField(); result = true;
modelFormField.getRequiredFieldStyle(); result = "someCssClass";
httpSession.getAttribute("delegatorName"); result = "DelegatorName";
}
};

final RenderableFtl renderableFtl = renderableFtlFormElementsBuilder.textField(Map.of("session", httpSession), textField, true);
assertThat(renderableFtl, MacroCallMatcher.hasName("renderTextField"));
assertThat(renderableFtl, MacroCallMatcher.hasParameters(MacroCallParameterMatcher.hasNameAndBooleanValue("required", true)));
assertThat(renderableFtl, MacroCallMatcher.hasParameters(MacroCallParameterMatcher.hasNameAndStringValue("className", "someCssClass required")));
}

@Test
public void textFieldPattern(@Mocked final ModelFormField.TextField textField) {
new Expectations() {
{
textField.getPattern(); result = "\\d{4,4}";
httpSession.getAttribute("delegatorName"); result = "DelegatorName";
}
};

final RenderableFtl renderableFtl = renderableFtlFormElementsBuilder.textField(Map.of("session", httpSession), textField, true);
assertThat(renderableFtl, MacroCallMatcher.hasName("renderTextField"));
assertThat(renderableFtl, MacroCallMatcher.hasParameters(MacroCallParameterMatcher.hasNameAndStringValue("pattern", "\\d{4,4}")));
}

@Test
public void textFieldPatternOnInvalidType(@Mocked final ModelFormField.TextField textField) {
new Expectations() {
{
textField.getPattern(); result = "\\d{4,4}";
textField.getType(); result = "number";
httpSession.getAttribute("delegatorName"); result = "DelegatorName";
}
};

textField.getPattern();
final RenderableFtl renderableFtl = renderableFtlFormElementsBuilder.textField(Map.of("session", httpSession), textField, true);

assertThat(renderableFtl, MacroCallMatcher.hasName("renderTextField"));
assertThat(renderableFtl, MacroCallMatcher.hasParameters(MacroCallParameterMatcher.hasNameAndStringValue("pattern", "")));
}

@Test
public void textareaFieldSetsIdValueLengthAndSize(@Mocked final ModelFormField.TextareaField textareaField) {
final int maxLength = 142;
Expand Down
4 changes: 2 additions & 2 deletions themes/common-theme/template/Login.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ under the License.
<table class="basic-table" cellspacing="0">
<tr>
<td class="label">${uiLabelMap.CommonUsername}</td>
<td><input type="text" name="USERNAME" value="${username}" size="20"/></td>
<td><input type="text" name="USERNAME" value="${username}" size="20" required/></td>
</tr>
<tr>
<td class="label">${uiLabelMap.CommonPassword}</td>
<td><input type="password" name="PASSWORD" autocomplete="off" value="" size="20"/></td>
<td><input type="password" name="PASSWORD" autocomplete="off" value="" size="20" required/></td>
</tr>
<#if ("Y" == useMultitenant) >
<#if !requestAttributes.userTenantId??>
Expand Down
Loading

0 comments on commit 488d1a0

Please sign in to comment.