TimeTextBox: Setting constraints does not update 'state' property

[Paul Christopher]

Description

If you e.g. specify certain constraints like a min time in the page's ready function, the TimeTextBox's get('state') function still returns an empty string, although the entry is now invalid. Therefore the whole form is reported as valid although it is not valid anymore: form.get('state') returns an empty string, too. Only calling form.validate() forces a recalculation of the state properties.

Steps to reproduce the issue

Run the attached test case. The form is - as it is - invalid. TimeTextBox's value is 9, constraints.min is set to 10 in ready().

Press button "form.get('state')". This will get the form state via the form.get('state') method. Notice: The form is reported as valid, although it is not valid.

Press button "TimeTextBox.get('state')". This will retrieve the valid state of the TimeTextBox using TimeTextBox.get('state'). Again: the control is reported as valid (=an empty string) although it is - with the new constraints given - not valid anymore.

Now press button "form.validate()". The form is now reported as invalid, the TimeTextBox highlighted red.

Discussion

This is a problem since form.js onSubmit is

onSubmit: function(/*Event?*/ /*===== e =====*/){
			// summary:
			//		Callback when user submits the form.
			// description:
			//		This method is intended to be over-ridden, but by default it checks and
			//		returns the validity of form elements. When the `submit`
			//		method is called programmatically, the return value from
			//		`onSubmit` is used to compute whether or not submission
			//		should proceed
			// tags:
			//		extension

			return this.isValid(); // Boolean
		}

and _formMixin.js isValid is

isValid: function(){
	 		// summary:
	 		//		Returns true if all of the widgets are valid.
			//		Deprecated, will be removed in 2.0.  Use get("state") instead.

			return this.state == "";
		}

Possible fix

Does ValidationTextBox.js _setConstraintsAttr simply needs calling isValid like so??

		_setConstraintsAttr: function(/*Object*/ constraints){
			if(!constraints.locale && this.lang){
				constraints.locale = this.lang;
			}
			this._set("constraints", constraints);
			this._computePartialRE();
			this.set('state', this.isValid());

		}

Or is it maybe better to have a custom getter for the state property?

[Paul Christopher]

I had some time to look at the code now. I think _setConstraintsAttr simply needs to call _refreshState (as _setRequiredAttr and _setDisabledAttr already does), see patch attached.

• _setValueAttr uses this.validate instead of this.refreshState. Is this correct?

• postMixinProperties sets constraints using "this._setConstraintsAttr(this.constraints)". I would have expected that "this.set('constraints', this.constraints)" is used. Is this correct?

• _TimePicker.js postMixinProperties method uses "this._setConstraintsAttr(this.constraints)" instead of(?) "this.set('constraints', this.constraints)", too. Correct?

[Paul Christopher]

Changed file extension from *.txt to *.diff

[Douglas Hays]

In [28743]:

Fixes #15432. Call _refreshState() from _setConstraintsAttr to set state. Added automated tests.

[bill]

Except that it causes a failure in the beforeFocus test of ValidationTextBox.html (tested on IE8).

_AssertFailure: assertEqual() failed:
	expected
		Incomplete
	but got
		

It also causes 3 failures in Form_state.html (again on IE8).

And finally, a failure in !FilteringSelect_mouse.html.

[Douglas Hays]

Fixes #15432. Run _refreshState in ValidationTextBox.js on startup() after all initial _set*Attr methods have run instead of after each.

[Douglas Hays]

In [28798]:

Fixes #15432. Run _refreshState in ValidationTextBox?.js on startup() after all initial _set*Attr methods have run instead of after each.

[bill]

In [28843]:

remove empty summary, refs #15432 !strict.

[Douglas Hays]

In [29071]:

Refs #15432. Need to call startup() on programmatically instantiated TextBbox? subclasses in order to set this._started which in turn allows this._refreshState() to run.

[ben hockey]

surely automatically calling startup is not right?

[Douglas Hays]

In [29076]:

Fixes #15432. Have _refreshState only validate after _created is set instead of _started since startup() is sometimes never called on TextBox? subclasses.