[Mifos-developer] Mifos forms swallow keystrokes

[Sam Birney]

Here is an odd presentation layer issue...  I noticed that when using various Mifos forms, for example when disbursing a loan, Mifos "swallows" some keystrokes.  So, for example, if I type a repayment amount of "123" quickly, then only the value "1" shows up in that field even after the browser has had sufficient time to process.  This is seriously going to frustrate the accountants here, who type numbers like lightning, and will possibly lead to serious financial errors.  I suspect the culprit is the javascripts which capture the keystrokes and then process them.  Has anyone else noticed this?  Anyone have ideas about what can be done?  I know, disable javascript, but may of those validators are actually useful/load-bearing :)

thanks,
Sam

[Adam Feuer]

> Here is an odd presentation layer issue...  I noticed that when using
> various Mifos forms, for example when disbursing a loan, Mifos "swallows"
> some keystrokes.

Sam,

Yeah, I've noticed that, especially when running Selenium scripts.
It's the old, ineffecient Javascript validation slowing things down.

Are you seeing this in manual data entry, or in Selenium? If only in
Selenium, there are ways to get around the problem.

-adam
--
Adam Feuer <adamf at pobox dot com>

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july

[Van Mittal-Henkle]

The plan is still to get rid of the javascript.  This will help support the move to internationalized numbers too.

 

Short of doing that, perhaps the javascript could be improved somehow.

 

--Van

[Sam Birney]

> Are you seeing this in manual data entry, or in Selenium? If only in
> Selenium, there are ways to get around the problem.

Strangely I have only noticed it with manual entry, Selenium seems to
do fine...

[Jeff Brewster]

Sam,
Sadly I type very slowly, so I hadn't noticed it but have just repro'ed
this on the Apply payment and Apply Charge pages. I've logged a bug-
https://mifos.dev.java.net/issues/show_bug.cgi?id=2561. If you have
other pages you've noted, please let me know.

Thanks,
Jeff

>-----Original Message-----
>From: Sam Birney [mailto:sam.b...@gmail.com]

[Sam Birney]

I also found it on the "amount" and "interest rate" fields of the loan
creation page. I tracked it down to MifosDecimalInput.java, a custom
jsp tag used for decimal text inputs. This file includes
conversion.js and injects the jsp with this:

<input type="text" name="chargeAmount" value="" onkeydown="saveText
(this);this.focus();" onkeyup="return chekDecimal(7.2,this, event); "
onkeypress="return keyPress(7.2,this,event);" onblur="if( false ==
doValidation(this,null,null,7.2)) this.focus();"
id="applyCharges.input.amount">

I looked at conversion.js but couldn't make much sense of it. I
noticed that by pasting in an invalid number, like the string "hello",
I can get around this validation because it relies on keys being
pressed. (That also explains why normal selenium scripts do not
encounter this, because when it uses the type command it does not
generate keystroke events.) When I put "hello" into the numeric field
I got a stacktrace NumberFormatException on the result page.

Getting a stacktrace actually seems better (safer) to me than silently
eating some of the numerals in the input, so what do you guys think
about removing this javascript as a short-term improvement?

thanks,
Sam

On Nov 6, 6:53 pm, "Jeff Brewster" <jbrews...@grameenfoundation.org>
wrote:

> Sam,
> Sadly I type very slowly, so I hadn't noticed it but have just repro'ed

> this on the Apply payment and Apply Charge pages.   I've logged a bug-https://mifos.dev.java.net/issues/show_bug.cgi?id=2561.   If you have

[Van Mittal-Henkle]

> I looked at conversion.js but couldn't make much sense of it. I
> noticed that by pasting in an invalid number, like the string "hello",
> I can get around this validation because it relies on keys being
> pressed. (That also explains why normal selenium scripts do not
> encounter this, because when it uses the type command it does not
> generate keystroke events.) When I put "hello" into the numeric field
> I got a stacktrace NumberFormatException on the result page.
>
> Getting a stacktrace actually seems better (safer) to me than silently
> eating some of the numerals in the input, so what do you guys think
> about removing this javascript as a short-term improvement?

I agree that the current java script is not good and we should remove
it. But right now some users are being prevented from entering bad data
and those users would instead see a stack trace. It seems like this
could lead to confusion or problems for those users (and it is
unfriendly to surface stack traces). I would propose replacing the
javascript with server side validation instead.

--Van

[Jeff Brewster]

Hey Sam,
I added your investigation details to the issue tracker issue - # 2561.

Thanks!
Jeff

[Sam Birney]

I agree, server-side validation is the way to go here and that
surfacing stack
traces is not desirable. I guess I was assuming we would not have
time to
add server-side validation before this deployment goes live, but that
commenting
out the javascript could be done in time. If we could have the server-
side
validation to fix this bug before we go live that would be ideal.

If not, I still think stack trace is better than silently removing
some of the numbers
that are typed. Either one is confusing, but the workaround for the
stack trace is
to press the back button, where as the workaround for entering a wrong
number
requires getting someone with permissions to reverse the payment and
then redo
the payment, which is inefficient, process-wise. Also, I suspect the
case where
a user enters a non-number into the number field is very rare, at
least here, maybe
less than 1 in 1000, whereas the case where they type numbers quickly
is all
the time.

You didn't mention which of those two you think is worse for users...

So, I totally agree about the long-term approach, but for the short-
term, I guess
I would ask can we fix the bug as you suggest before the deployment?
Or if not,
can we remove this javascript before the deployment? Maybe a flag to
disable
it if you think other Mifos deployments would rather have the
javascript?

thanks,
Sam

On Nov 9, 7:35 pm, "Van Mittal-Henkle" <va...@grameenfoundation.org>
wrote:

[Jeff Brewster]

>So, I totally agree about the long-term approach, but for the short-
>term, I guess
>I would ask can we fix the bug as you suggest before the deployment?
>Or if not,
>can we remove this javascript before the deployment? Maybe a flag to
>disable
>it if you think other Mifos deployments would rather have the
>javascript?
>

Sam,
I feel that if the bug can't be fixed in the short term, we should leave
the behavior as-is and not show a stack trace. That opinion is based on
lack of feedback from other MFI's about this field swallowing keystrokes
over the past hours and hours of data entry. For people who don't do
coding, I think a stack trace is disorienting and alarming. I know I
watch the screen when I enter numeric values in my banking site, etc. so
I suspect the people doing data entry are being diligent about the
number being entered and self-correct any swallowed keystrokes.

Of course, if we can make it work perfectly, then we don't have to chose
between two undesirable paths!

Jeff

[Sam Birney]

I think this issue is really going to make the accountants here unsure
of Mifos, and will probably lead to several incorrect amounts being
entered. So, I started working on a patch. I found that if I put the
following code snippet in ApplyChargeActionForm.validate, for example,
it validates the amount field and shows the red error dot on non-
numeric input instead of leading to a stack trace. We can continue on
this path, externalizing the strings and making the proper error
message show up and so on, but first I just wanted to check and see if
there is still time for this in Gazelle B, or maybe as a hot-fix
thereafter?

<code>
try {
Double d = getDoubleValue(getChargeAmount());
} catch (Exception e) {
errors.add("applyCharges.error.message",
new ActionMessage("bad number format"));
}
</code>

On Nov 12, 12:49 am, "Jeff Brewster" <jbrews...@grameenfoundation.org>
wrote:

[Adam Monsen]

> but first I just wanted to check and see if
> there is still time for this in Gazelle B, or maybe as a hot-fix
> thereafter?

Work is progressing on isssue 2561. We're ripping out much of the broken
JavaScript validation from the JSPs for decimal input and replacing it
with server-side validation. The fixes are currently on
branches/issue_2561, but we'll make the call tomorrow to see what makes
it into the next release (version 1.4) and what will come later.

[Orange Mile]

You could stick with javascript just not the awfully complicated on key stroke validation. For example, a simple onsubmit or onBlur out to do the trick.

Some java script code here:
http://stackoverflow.com/questions/18082/validate-numbers-in-javascript-isnumeric

Another option would be to use jquery with a validation plug-in like:
http://jquery.com/
http://plugins.jquery.com/project/ufvalidator
http://plugins.jquery.com/project/Plugins/category/20

Hope this helps.

Regards,
Vadim

[Adam Monsen]

> You could stick with javascript just not the awfully complicated on
> key stroke validation.

Agreed.

The first step is just removing this broken code, then we can look to
ways to properly implement JavaScript validation, if that is
required/desired.

If we do opt for JavaScript validation, I would vote that the same
validation rules are used as those on the server side, but also that the
rules would be maintainable in one place, ie: shared, not duplicated.