#635 Perpetual TValidator error loop

[Vidar Hasfjord]

In some circumstances, two TEdit fields in a dialog, both using a TValidator, may cause a perpetual error message loop, directing the focus back and forth between the fields.

To reproduce: Build and run the Classes example in OWLNext 7.0.19, open the TValidate example, clear the "Name" field by pressing Delete/Backspace, then press Alt+A to move to the "Address" field. Error message "Non-blank input is required" is shown. Close it. The message shows again. Ad infinitum.

The general problem was noted by Joe Slater when working on validation [feature-requests:#77]:

See: https://sourceforge.net/p/owlnext/feature-requests/77/#750f/bc1d

The same ticket discusses my simplified (non-coercive) validation solution in Owlet. This solution does not filter input, force focus, nor generate error messages until the user commits the dialog. Instead, invalid fields are indicated visually. Feel free to take a closer look.

See: https://sourceforge.net/p/owlnext/code/8602/tree/branches/owlet/examples/validate

[Ognyan Chernokozhev]

Yep, noticed that as well.

[Ognyan Chernokozhev]

Still trying to wrap my head around the code, but it seems that after the edit control with invalid data loses focus, sets ValidatorReFocus to itself and posts WM_CHILDINVALID, but in the meantime there are calls to TEdit::IsValid(false) where the TRefocusGuard clears the ValidatorReFocus and thus cause the validation to get in the oscillation loop.

[Vidar Hasfjord]

@jogybl wrote:

Still trying to wrap my head around the code

Yeah, it hurts. Unfortunately, I had to go through it twice. First time, I ended up simplifying it all in Owlet (error messages only produced at commit, invalid fields indicated visually). Unfortunately, I had to go through it a second time, reviewing Joe Slater's work, as he proceeded to enhance the old ugly mess. In retrospect, I should have forced through the saner Owlet solution for version 7. Feel free to do so now for version 8.

PS. Joe linked a couple of articles by Raymond Chen on the issue, in which he was very critical on coercive validation and recommended against it, I seem to remember. Unfortunately, the links are broken, and a quick search didn't find them.

[Ognyan Chernokozhev]

For the moment, I have pushed a quick fix for the case of when exactly to set ValidatorReFocus to null, which seems to fix the loop.

The same issue would be present in 7.0, right?

And for the future I agree that displaying message boxes on losing focus is too obtrusive, and just visually indicating the invalid fields is better user experience.

Also I wonder if the current approach works correctly in different cases, like if the edit controls are in a wizard dialog page, which may not have IDOK button.

[Vidar Hasfjord]

@jogybl wrote:

For the moment, I have pushed a quick fix [r8603]

Good! Seems to work.

The same issue would be present in 7.0, right?

I would think so. Feel free to take ticket ownership and target this fix for version 7. Then it should be added to the list of upcoming fixes in OWLNext Releases.

(Edit: I've now tested to confirm this, and yes, it is present in 7.0.19. For future reference, I've updated the ticket description to use this version to reproduce the issue. Note that I've also tested 6.44, and couldn't reproduce the issue, meaning it may be a regression in 7.0 [feature-requests:#77].)

By the way, I added a banner hint with a valid dummy number for your new credit card field in the TValidate example ("TValidate" is a typo; should be "TValidator", right?), so that the test user can proceed without a real credit card number. I also resolved the mnemonic accelerator conflict (Alt+C). See [r8605].

Also I wonder if the current approach works correctly in different cases, like if the edit controls are in a wizard dialog page, which may not have IDOK button.

Good point. The Owlet solution circumvents all these issues.

[Ognyan Chernokozhev]

Fixed in trunk in [r8603] and in 7.0 branch in [r8606].