Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

fix #303 Infinite loop on datepicker #308

Closed
wants to merge 1 commit into from

2 participants

@piuccio

I believe this fixes the datepicker issue.

:warning: It would be nice to add a couple of unit tests.
Cases like binding both value and invalid text or binding an incorrect value are not properly tested.

And who knows what happens when you have both prefill and invalidText!

@piuccio

This amended commit adds some tests on invalid text + prefill + helptext

@divdavem
Collaborator

Integrated in 027a79e

@divdavem divdavem closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 8, 2013
  1. @piuccio
This page is out of date. Refresh to see the latest.
View
12 src/aria/widgets/form/Select.js
@@ -188,9 +188,9 @@ Aria.classDefinition({
this._updateState();
if (this._cfg.formatError && this._cfg.validationEvent === 'onBlur') {// show
// errortip on blur used for debug purposes
- this._validationPopupShow(this);
+ this._validationPopupShow();
} else { // dispose of error tip
- this._validationPopupHide(this);
+ this._validationPopupHide();
if (this._cfg.directOnBlurValidation) {
if (this._cfg.bind) {
var bind = this._cfg.bind.value;
@@ -219,7 +219,7 @@ Aria.classDefinition({
if (this._cfg) {
if (this._cfg.validationEvent === 'onFocus'
&& ((this._cfg.formatError && this._cfg.formatErrorMessages.length) || (this._cfg.error && this._cfg.errorMessages.length))) {
- this._validationPopupShow(this);
+ this._validationPopupShow();
}
}
this._updateState();
@@ -231,9 +231,9 @@ Aria.classDefinition({
}
this.evalCallback(this._cfg.onvalidate);
if (arg.popup && (this._cfg.error)) {// show errortip onfocus
- this._validationPopupShow(this);
+ this._validationPopupShow();
} else { // dispose of error tip
- this._validationPopupHide(this);
+ this._validationPopupHide();
}
},
@@ -318,7 +318,7 @@ Aria.classDefinition({
this.changeProperty("error", false);
if (!(this._cfg.formatError && this._cfg.formatErrorMessages.length)
|| (this._cfg.error && this._cfg.errorMessages.length)) {
- this._validationPopupHide(this);
+ this._validationPopupHide();
}
if (this._cfg.onchange) {
this.evalCallback(this._cfg.onchange);
View
234 src/aria/widgets/form/TextInput.js
@@ -197,40 +197,97 @@ Aria.classDefinition({
},
/**
- * Internal method to process the input block markup inside the frame
- * @param {aria.templates.MarkupWriter} out the writer Object to use to output markup
- * @protected
+ * Get the text value of the input field. If available it tries to use the internal valid value, otherwise uses
+ * the invalid text. If none of them is a non empty string it return the prefilled value. This method doesn't
+ * handle helptext, as this value is not just text but also style.
+ * @return {String}
*/
- _inputWithFrameMarkup : function (out) {
- var cfg = this._cfg, skinObj = this._skinObj, hts = this._helpTextSet, htc = this._skinObj.helpText, color = this._getTextFieldColor(), report;
- var stringUtils = aria.utils.String;
+ _getText : function () {
+ var cfg = this._cfg;
- var inlineStyle = ['padding:', skinObj.innerPaddingTop, 'px ', skinObj.innerPaddingRight, 'px ',
- skinObj.innerPaddingBottom, 'px ', skinObj.innerPaddingLeft, 'px;position:relative;margin:0;'];
- if (!this._simpleHTML) {
- inlineStyle.push("background-color:transparent;border-width:0px;vertical-align:top;");
+ // invalid text and value shouldn't be set at the same time
+ var text = cfg.invalidText || "";
+
+ if (text && cfg.value) {
+ // There's both a value and an invalid text, prefer the value
+ this.setProperty("invalidText", null);
+ text = "";
}
- // check value to set appropriate state and text
- var text = cfg.invalidText || "";
+ // Validate the value in the configuration
var res = this.checkValue({
text : text,
value : cfg.value,
performCheckOnly : true
});
+
if (res.report) {
- report = res.report;
+ var report = res.report;
if (!text && report.text != null) {
- text = '' + report.text; // String cast
+ text = '' + report.text; // String cast of valid value
}
report.$dispose();
}
if (!text) {
- var prefillText = (cfg.prefill && this.controller && this.controller.getDisplayTextFromValue)
- ? this.controller.getDisplayTextFromValue(cfg.prefill)
- : cfg.prefill;
- text = prefillText;
+ text = this._getPrefilledText(cfg.prefill);
+ }
+
+ return text;
+ },
+
+ /**
+ * Set a given text as value for the text input. This method handles helptext for non password fields.
+ * @param {String} value Text to be set, if empty uses the value from <code>this._getText</code> or the
+ * helptext
+ */
+ _setText : function (value) {
+ if (value == null) {
+ value = this._getText();
+ }
+
+ // _getText only handles valid / invalid values and prefills, not the helptext
+ if (!value && !this._isPassword) {
+ // We don't want to handle helptext in password fields, first remove any text
+ this.getTextInputField().value = "";
+ this.setHelpText(true);
+ } else if (value) {
+ this.getTextInputField().value = value;
+ }
+ },
+
+ /**
+ * Get the text value that should be prefilled in the widget. It takes a default value that could be anything
+ * (like dates). For non string objects it delegates the string resolution to the controller.
+ * @param {Object} value
+ * @return {String}
+ * @protected
+ */
+ _getPrefilledText : function (value) {
+ if (value && this.controller && this.controller.getDisplayTextFromValue) {
+ return this.controller.getDisplayTextFromValue(this._cfg.prefill);
+ } else {
+ return value;
+ }
+ },
+
+ /**
+ * Internal method to process the input block markup inside the frame
+ * @param {aria.templates.MarkupWriter} out the writer Object to use to output markup
+ * @protected
+ */
+ _inputWithFrameMarkup : function (out) {
+ var cfg = this._cfg, skinObj = this._skinObj, hts = this._helpTextSet, htc = this._skinObj.helpText, color = this._getTextFieldColor();
+ var stringUtils = aria.utils.String;
+
+ var inlineStyle = ['padding:', skinObj.innerPaddingTop, 'px ', skinObj.innerPaddingRight, 'px ',
+ skinObj.innerPaddingBottom, 'px ', skinObj.innerPaddingLeft, 'px;position:relative;margin:0;'];
+ if (!this._simpleHTML) {
+ inlineStyle.push("background-color:transparent;border-width:0px;vertical-align:top;");
}
+
+ // check value to set appropriate state and text
+ var text = this._getText();
+
if (hts) {
// FIXME : re-activate helpText in password field in IE
if (this._isIE7OrLess && this._isPassword) {
@@ -309,8 +366,9 @@ Aria.classDefinition({
},
/**
- * Check that the value displayed in the field is correct If not, set the field in error
- * @param {JSON cfg} arg - optional arguments to control the check behavior
+ * Check that the value displayed in the field is correct. If not, set the field in error and store its invalid
+ * text
+ * @param {Object} arg - optional arguments to control the behavior
*
* <pre>
* {
@@ -318,7 +376,17 @@ Aria.classDefinition({
* value: {Object} - internal widget value,
* performCheckOnly: {Boolean} - perfom only value/text check do not update th widget display,
* resetErrorIfOK: {Boolean} (default:true) - tells if error display must be removed if check is OK
- * (usefull when check is done on 'strange' events like mouseover }
+ * (usefull when check is done on 'strange' events like mouseover)
+ * }
+ * </pre>
+ *
+ * @return {Object}
+ *
+ * <pre>
+ * {
+ * isValid : {Boolean} Whether the value is valid or not
+ * report : {Object} Controller's report, if any
+ * }
* </pre>
*/
checkValue : function (arg) {
@@ -338,60 +406,55 @@ Aria.classDefinition({
this.changeProperty("formatError", false);
}
- if (this.controller) {
+ var result = {
+ isValid : true,
+ report : null
+ };
- var hasErrors = (this._cfg.formatErrorMessages.length ? true : false);
- var report;
- // if (value != null || performCheckOnly) {
- if (value != null) {
- report = this.controller.checkValue(value);
- } else {
- report = this.controller.checkText(text, hasErrors);
- }
+ if (!this.controller) {
+ // There's no controller so we assume the value to be valid
+ return result;
+ }
- if (report) {
- if (report.errorMessages.length) {
- this.changeProperty("invalidText", text);
- if (this._cfg.directOnBlurValidation && !performCheckOnly) {
- this.changeProperty("formatErrorMessages", report.errorMessages);
- }
- } else if (this._cfg.formatError === false
- && (aria.utils.Type.isArray(this._cfg.formatErrorMessages) && this._cfg.formatErrorMessages.length)) {
- this.changeProperty("invalidText", null);
- this.changeProperty("formatErrorMessages", []);
- } else {
- if (report.ok) {
- this.changeProperty("invalidText", null);
- }
- }
- if (performCheckOnly) {
- return {
- isValid : report.ok,
- report : report
- };
- } else {
- this._reactToControllerReport(report, arg);
- return;
+ var hasErrors = (this._cfg.formatErrorMessages.length ? true : false);
+ var report;
+
+ if (value != null) {
+ report = this.controller.checkValue(value);
+ } else {
+ report = this.controller.checkText(text, hasErrors);
+ }
+
+ if (!report) {
+ // No report means that the controller is handling the value asynchronously, consider it as valid
+ return result;
+ }
+
+ if (report.errorMessages.length) {
+ if (!performCheckOnly) {
+ this.changeProperty("value", null);
+ this.changeProperty("invalidText", text);
+ if (this._cfg.directOnBlurValidation) {
+ this.changeProperty("formatErrorMessages", report.errorMessages);
}
- } else {
- return {
- isValid : true,
- report : null
- };
}
+ } else if (this._cfg.formatError === false && aria.utils.Type.isArray(this._cfg.formatErrorMessages)
+ && this._cfg.formatErrorMessages.length) {
+ this.changeProperty("invalidText", null);
+ this.changeProperty("formatErrorMessages", []);
+ } else if (report.ok && !performCheckOnly) {
+ this.changeProperty("invalidText", null);
+ }
- } else {
+ if (performCheckOnly) {
return {
- isValid : true,
- report : null
+ isValid : report.ok,
+ report : report
};
+ } else {
+ this._reactToControllerReport(report, arg);
+ return;
}
-
- return {
- isValid : true,
- report : null
- };
-
},
/**
@@ -580,7 +643,7 @@ Aria.classDefinition({
return;
}
- // in case things have change the field, try to set an helptext
+ // in case things have changed the field, try to set an helptext
this.setHelpText(true);
if (((aria.utils.Type.isArray(cfg.value) && aria.utils.Array.isEmpty(cfg.value)) || !cfg.value)
&& cfg.prefill && cfg.prefill + "") {
@@ -606,7 +669,7 @@ Aria.classDefinition({
if (!res || !res.isValid) {
var textField = this.getTextInputField();
if (textField) {
- textField.value = newValue;
+ this._setText(newValue);
}
}
if (res && res.report) {
@@ -732,9 +795,9 @@ Aria.classDefinition({
}
if (arg.popup && (this._cfg.error)) {
- this._validationPopupShow(this);
+ this._validationPopupShow();
} else {
- this._validationPopupHide(this);
+ this._validationPopupHide();
}
},
@@ -817,7 +880,7 @@ Aria.classDefinition({
cfg = this._cfg
if (cfg.validationEvent === 'onFocus'
&& ((cfg.formatError && cfg.formatErrorMessages.length) || (cfg.error && cfg.errorMessages.length))) {
- this._validationPopupShow(this);
+ this._validationPopupShow();
}
}
this._updateState();
@@ -871,10 +934,10 @@ Aria.classDefinition({
if (cfg.formatError && cfg.validationEvent === 'onBlur') {
// show errortip on blur used for debug purposes
- this._validationPopupShow(this);
+ this._validationPopupShow();
} else {
// dispose of error tip
- this._validationPopupHide(this);
+ this._validationPopupHide();
if (cfg.directOnBlurValidation) {
if (cfg.bind) {
var bind = cfg.bind.value;
@@ -914,18 +977,12 @@ Aria.classDefinition({
_setAutomaticBindings : function (cfg) {
this.$InputWithFrame._setAutomaticBindings.call(this, cfg);
var value = null, prefill = null, metaDataObject;
- if (cfg) {
- if (cfg.bind) {
- value = cfg.bind.value; // get any binding on the value
- // property
- prefill = cfg.bind.prefill; // get any binding on the
- // prefill property
- }
+ if (cfg && cfg.bind) {
+ value = cfg.bind.value;
+ prefill = cfg.bind.prefill;
}
- if (value && value.inside) { // only add the meta data convention
- // if a value property has been
- // bound
+ if (value && value.inside) {
metaDataObject = aria.utils.Data._getMeta(value.inside, value.to, false);
if (!cfg.bind.invalidText) {
cfg.bind.invalidText = {
@@ -934,10 +991,7 @@ Aria.classDefinition({
};
}
}
- if (prefill && prefill.inside) { // only add the meta data
- // convention if a prefill
- // property has been
- // bound
+ if (prefill && prefill.inside) {
metaDataObject = aria.utils.Data._getMeta(prefill.inside, prefill.to, false);
if (!cfg.bind.prefillError) {
cfg.bind.prefillError = {
@@ -1085,9 +1139,7 @@ Aria.classDefinition({
if (value == null) {
prefillText = "";
} else {
- prefillText = (this.controller && this.controller.getDisplayTextFromValue)
- ? this.controller.getDisplayTextFromValue(value)
- : value;
+ prefillText = this._getPrefilledText(value);
}
if (cfg.prefillError) {
prefillText = "";
View
1  test/aria/widgets/WidgetsTestSuite.js
@@ -41,5 +41,6 @@ Aria.classDefinition({
this.addTests("test.aria.widgets.form.TextareaTest");
this.addTests("test.aria.widgets.form.TextInputTest");
this.addTests("test.aria.widgets.form.multiselect.issue223.MultiSelect");
+ this.addTests("test.aria.widgets.form.datepicker.DatePickerTestSuite");
}
});
View
9 test/aria/widgets/form/datepicker/DatePickerTestSuite.js
@@ -0,0 +1,9 @@
+Aria.classDefinition({
+ $classpath : "test.aria.widgets.form.datepicker.DatePickerTestSuite",
+ $extends : "aria.jsunit.TestSuite",
+ $constructor : function () {
+ this.$TestSuite.constructor.call(this);
+
+ this.addTests("test.aria.widgets.form.datepicker.issue303.InfiniteLoop");
+ }
+});
View
249 test/aria/widgets/form/datepicker/issue303/InfiniteLoop.js
@@ -0,0 +1,249 @@
+Aria.classDefinition({
+ $classpath : "test.aria.widgets.form.datepicker.issue303.InfiniteLoop",
+ $extends : "aria.jsunit.WidgetTestCase",
+ $dependencies : ["aria.widgets.form.DatePicker", "aria.utils.Date"],
+ $prototype : {
+ tearDown : function () {
+ aria.jsunit.helpers.OutObj.clearAll();
+ },
+
+ createInstance : function (cfg) {
+ // Ignore most of the options because those are normalized from other widgets
+ var config = {
+ bind : cfg.bind,
+ helptext : cfg.helptext
+ };
+ // doing a json.clone won't work because it'll recreate the datamodel
+
+ var widget = new aria.widgets.form.DatePicker(config, aria.jsunit.helpers.OutObj.tplCtxt);
+
+ // Prevent the popup from opening
+ widget._validationPopupShow = Aria.empty;
+
+ return widget;
+ },
+
+ initializeWidgets : function (/* list of widgets*/) {
+ var widget;
+ for (var i = 0; i < arguments.length; i += 1) {
+ widget = arguments[i];
+ widget.writeMarkup(this.outObj);
+ }
+
+ this.outObj.putInDOM();
+
+ for (i = 0; i < arguments.length; i += 1) {
+ widget = arguments[i];
+ widget.initWidget();
+ widget.initWidgetDom();
+ }
+ },
+
+ /**
+ * There used to be an infinite loop when setting an invalid text on a datepicker bound to the same value as
+ * another datepicker. With this test I recreate the issue, set the value and assert that nothing bad happens
+ */
+ testInfiniteLoop : function () {
+ var today = new Date();
+ var datamodel = {
+ value : today
+ };
+
+ var cfg = {
+ bind : {
+ value : {
+ inside : datamodel,
+ to : "value"
+ }
+ }
+ };
+
+ var widget = this.createInstance(cfg);
+ var otherListener = this.createInstance(cfg);
+ this.initializeWidgets(widget, otherListener);
+
+ var input = widget.getTextInputField();
+
+ var formatted = aria.utils.Date.format(today, widget.controller._pattern);
+ this.assertEquals(input.value, formatted, "Input value should be " + formatted + " got " + input.value);
+
+ // Now simulate a type
+ try {
+ widget._dom_onfocus();
+ input.value = "invalid text";
+ widget._dom_onblur();
+ } catch (ex) {
+ this.$logError("Changing the value shouldn't throw.\nError message: %1", [ex.message], ex);
+ }
+
+ widget.$dispose();
+ otherListener.$dispose();
+
+ this.assertLogsEmpty();
+ },
+
+ /**
+ * This test verifies that two datepicker bound both to value and invalid text work as expected. At any stage
+ * they should have the same value and invalid text is more important than value. It should be kept even after
+ * typing on a field that has already a value (it's old)
+ */
+ testDualBinding : function () {
+ var today = new Date();
+ var datamodel = {
+ value : today,
+ invalid : "wrong"
+ };
+
+ var cfg = {
+ bind : {
+ value : {
+ inside : datamodel,
+ to : "value"
+ },
+ invalidText : {
+ inside : datamodel,
+ to : "invalid"
+ }
+ }
+ };
+
+ var widget = this.createInstance(cfg);
+ var other = this.createInstance(cfg);
+ this.initializeWidgets(widget, other);
+
+ var inputOne = widget.getTextInputField();
+ var inputTwo = other.getTextInputField();
+
+ var formatted = aria.utils.Date.format(today, widget.controller._pattern);
+ this.assertEquals(inputOne.value, formatted, "Input value 1 should be " + formatted + " got '"
+ + inputOne.value + "'");
+ this.assertEquals(inputTwo.value, formatted, "Input value 2 should be " + formatted + " got '"
+ + inputTwo.value + "'");
+
+ // Now simulate a type
+ widget._dom_onfocus();
+ inputOne.value = "invalid text";
+ widget._dom_onblur();
+
+ this.assertEquals(inputTwo.value, "invalid text", "Input value 3 should be 'invalid text' got '"
+ + inputTwo.value + "'");
+
+ var tomorrow = aria.utils.Date.interpret("+1");
+ var formattedTomorrow = aria.utils.Date.format(tomorrow, widget.controller._pattern);
+
+ widget._dom_onfocus();
+ inputOne.value = formattedTomorrow;
+ widget._dom_onblur();
+
+ // Input two should just be the same
+ this.assertEquals(inputOne.value, formattedTomorrow, "Input value 4 should be " + formattedTomorrow
+ + " got '" + inputOne.value + "'");
+ this.assertEquals(inputTwo.value, formattedTomorrow, "Input value 5 should be " + formattedTomorrow
+ + " got '" + inputTwo.value + "'");
+ // Date might differ on the time
+ this.assertTrue(aria.utils.Date.isSameDay(datamodel.value, tomorrow), "Value in data model is not tomorrow");
+
+ widget.$dispose();
+ other.$dispose();
+ },
+
+ /**
+ * This test verifies that the prefill is added correctly on both datepicker. Prefill is bindable and should be
+ * used when the value is empty
+ */
+ testInvalidTextAndPrefill : function () {
+ var today = new Date();
+ var tomorrow = aria.utils.Date.interpret("+1");
+ var datamodel = {
+ value : today,
+ invalid : "wrong",
+ prefill : tomorrow
+ };
+
+ var cfg = {
+ bind : {
+ invalidText : {
+ inside : datamodel,
+ to : "invalid"
+ },
+ prefill : {
+ inside : datamodel,
+ to : "prefill"
+ }
+ }
+ };
+
+ var widget = this.createInstance(cfg);
+ var other = this.createInstance(cfg);
+ this.initializeWidgets(widget, other);
+
+ var inputOne = widget.getTextInputField();
+ var inputTwo = other.getTextInputField();
+
+ var formatted = aria.utils.Date.format(tomorrow, widget.controller._pattern);
+
+ // I expect to see the invalidText
+ this.assertEquals(inputOne.value, "wrong", "Input value should be 'wrong' got '" + inputOne.value + "'");
+ this.assertEquals(inputTwo.value, "wrong", "Input value should be 'wrong' got '" + inputTwo.value + "'");
+
+ // Now simulate canceling everything
+ widget._dom_onfocus();
+ inputOne.value = "";
+ widget._dom_onblur();
+
+ this.assertEquals(inputOne.value, formatted, "Input value should be '" + formatted + "' got '"
+ + inputOne.value + "'");
+ this.assertEquals(inputTwo.value, formatted, "Input value 2 should be '" + formatted + "' got '"
+ + inputTwo.value + "'");
+
+ widget.$dispose();
+ other.$dispose();
+ },
+
+ /**
+ * This test verifies the presence of help text in both datepickers. The helptext should be used when there's no
+ * value
+ */
+ testHelpText : function () {
+ var today = new Date();
+ var datamodel = {
+ value : today,
+ invalid : "wrong"
+ };
+
+ var cfg = {
+ bind : {
+ invalidText : {
+ inside : datamodel,
+ to : "invalid"
+ }
+ },
+ helptext : "help me!"
+ };
+
+ var widget = this.createInstance(cfg);
+ var other = this.createInstance(cfg);
+ this.initializeWidgets(widget, other);
+
+ var inputOne = widget.getTextInputField();
+ var inputTwo = other.getTextInputField();
+
+ // I expect to see the invalidText
+ this.assertEquals(inputOne.value, "wrong", "Input value should be 'wrong' got '" + inputOne.value + "'");
+ this.assertEquals(inputTwo.value, "wrong", "Input value should be 'wrong' got '" + inputTwo.value + "'");
+
+ // Now simulate canceling everything
+ widget._dom_onfocus();
+ inputOne.value = "";
+ widget._dom_onblur();
+
+ this.assertEquals(inputOne.value, "help me!", "Input value 1 should be 'help me!' got '" + inputOne.value
+ + "'");
+ this.assertEquals(inputTwo.value, "help me!", "Input value 2 should be 'help me!' got '" + inputTwo.value
+ + "'");
+
+ widget.$dispose();
+ other.$dispose();
+ }
+ }
+});
Something went wrong with that request. Please try again.