Decimal fields accept any combination of , . - as inputs but submit empty

[vituslehner]

Describe the bug Field type decimal accepts values with thousands seperators (like 123.123.123) but will not submit any value to the ODK server (field arrives empty and is shown as empty in e.g. ODK Central). The field also accepts wild . and , combinations like 12.212,12 or 123,123.12 or even 123,.,123. Enketo does not consider this as invalid or warn anyhow about it and all these combinations will arrive empty at the server.

This seems to be a Firefox peculiarity. Still, I think this should be considered as invalid by Enketo. See this warning on MDM: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/number#using_number_inputs

See some combinations in attached screenshot (my locale uses comma as decimal seperator).

To Reproduce See above.

Expected behavior Only decimals with a single "." or "," as decimal seperator should be considered as valid. For other values, the field should visibally be invalid.

Browser and OS (please complete the following information):

• Enketo 2.7.3
• Latest Firefox 89 on Windows 10
• Chrome does not seem to be affected

Additional context Find minimal XlsForm for tests attached. decimal-seperator-test.xlsx

[mattelacchiato]

I was able to reproduce this in Chrome with the value of 3-2. I guess, Enketo would need to add own validation and not rely on the browser validation. Only. I would like to add a PR for this.

[mattelacchiato]

Only decimals with a single "." or "," as decimal seperator should be considered as valid. For other values, the field should visibally be invalid.

The order of . and , is important, depending on what your decimal and thousand delimiter is. E.g. "12,345.678" vs "12.345,678": Both values could be legit, but guessing what the user's intend is not that easy. We could derive this from the browser primary language, but it's not granted that the user knows about this when entering its number.

[mattelacchiato]

It seems that enketo sets the value internally to "" (empty string), but the user sees its entered value. I would rephrase the Expected behavior: If the users enters valid values for enketo, those values should be exported. If a value is invalid, enketo should notify the user about this.

[mattelacchiato]

New observation: It seems to be chrome, which removes the value from the input element, but keeping the input displayed:

To reproduce in on a completely enketo-independent website:

1. create a number input field like <input type="number" id="foobar123">
2. enter valid number
3. document.getElementById("foobar123").value returns this number
4. enter invalid number (e.g. "1-3")
5. document.getElementById("foobar123").value returns empty string, but "1-3" is still displayed.

[mattelacchiato]

Setting the value to "" seems to be part of the HTML5 specification. The problem is the difference between what is displayed to the user and what is internally submitted to the server...

See this bug report: https://bugs.chromium.org/p/chromium/issues/detail?id=178437

[mattelacchiato]

@MartijnR I would like to propose a PR for this. I would ask for input.validity as described here, but I can't find this in the internal model (probably it's based on the XML document and not the HTML document). So I would have to query the HTML document, unless you have a better idea?

[MartijnR]

Setting the value to "" seems to be part of the HTML5 specification. The problem is the difference between what is displayed to the user and what is internally submitted to the server...

I agree. Funnily, comment 10 in that wontfix Chromium thread is mine.

Feel free to give the FF workaround a go in a PR. The current approach is in mask.js. I believe I gave up on the input.validity approach myself, but I don't remember why.

[mattelacchiato]

But we need a fix for Chrome, too. Does this masking-fix only work for FF?

Otherwise I would suggest to mark a field as invalid if input.validity.badInput == true. What do you think?

[MartijnR]

What do you think?

If it works, great. Try it out. It's been many years since I tried myself and browsers have evolved since then.

[mattelacchiato]

Okay, I've manged to get started with enketo development. After digging and thinking I could provide three different solutions:

1. on blur event: Set the input field to its internal value, so the field will become "" if invalid.
2. on blur event: Evaluate browser validation result (using input.validity)
3. disable browser-based validation completely via novalidate in the <form> tag.

Do we nee the browser-based validation at all? If not, i would prefer option 3. WDYT, @MartijnR ?

[mattelacchiato]

Oh, short update: novalidate="novalidate" is already set, but doesn't have any effect on this bug. I've also tested a single novalidate without success.

[MartijnR]

sorry for the late response @mattelacchiato (Note, I'm just one Enketo's devs now and somebody else may review a PR).

When looking in mask.js (around here), it looks like this is already doing what you suggest in (2, and 1) or isn't it?

[vituslehner]

So I just noticed something beyond my previous report which makes the issue more serious for me.

The behaviour is different depending on the locale that it configured in Firefox.

When set to German (where the decimals are typically seperated by comma) the behaviour is:

• 1.23 -> submitted as 1.23
• 1,23 -> submitted as 1.23

When set to English (US) (where decimals are typically separated by a point) the behaviour is:

• 1.23 -> submitted as 1.23
• 1,23 -> submitted empty

Last case submits empty also for required fields.

If the user is not aware of the browser language or the issue, this might lead to serious data loss. I would really appreciate help here. @mattelacchiato Do you think that your PR covers that too?

[mattelacchiato]

It should have the same functionality as in https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/number#value: No thousand separator allowed. Only the browser's language depended decimal separator is allowed. Otherwise it is marked as faulty.

But this is not tested, just assumed.

[vituslehner]

Thanks for your clarification! I will attempt to test it soon!

[vituslehner]

Dear Enketo team, I would be glad if someone could give this a review. We are actually loosing data by this bug and people can't even notice it because during submit, the form is filled, but the server receives empty. I would much appreciate it!

[lognaturel]

Thanks for the nudge. It's on the list and we'll try to get to it soon. Did you get a chance to follow up on https://github.com/enketo/enketo-core/issues/792#issuecomment-998944407?

[alxndrsn]

It looks like @mattelacchiato's PR takes proposed approach 3:

on blur event: Set the input field to its internal value, so the field will become "" if invalid.

This seems like an immediate improvement, even if not dream UX, so should probably be accepted.

There would still be the issue that a user could fill the field with an invalid value, and submit the form. If the field is not required, the value may the be thrown away and the form submitted. This would be surprising, and probably not visible to the user.

Also, if the user does not understand why the entered value is invalid, it will be non-obvious why the field keeps on emptying on blur.

A low-tech fix for both of these could be to replace event.target.value = ''; with e.g.:

if(confirm('You have entered an invalid value.  Is it OK to discard the value?')) {
  event.target.value = '';
} else {
  event.preventDefault();
  event.target.focus();
}

[eyelidlessness]

I discussed this with @lognaturel yesterday. We agreed that it makes more sense to treat invalid input as a validation error rather than implicitly setting the model value to blank and ignoring the input. That would be consistent with behavior for invalid values for other types.

My instinct is that the proper solution is likely easier than any kind of stopgap. This would involve:

• Replacing the keydown/paste/blur event handlers in mask.js with an input handler with behavior consistent with Chrome (because that behavior is exactly what I expect for a number field). This handler should be sufficient to cover all the cases mask.js intends to address, and can do
• Using the pattern attribute to enforce validity. According to the FAQ, we no longer need to worry about supporting IE 11, and all of our supported browsers do support pattern. If there's objection to dropping that particular backwards compatibility we could certainly use a polyfill.
• Insofar as invalid values may be entered in ways we haven't anticipated, preserve the user-entered value in the DOM as we presently do, and store either the same string value (or NaN) in the model.
• Validate that the invalid value (or NaN) is handled as expected when the value is referenced in XPath expressions. Specifically it should not produce runtime errors which cause the form to enter an inconsistent state, and should produce NaN in number-returning expressions.

I realize this list is long, but I believe it should be pretty quick fix and would also reduce complexity. We could also take this opportunity to also address #484, using Intl.NumberFormat to determine the appropriate decimal character to use in the input handler and the pattern attribute.

[eyelidlessness]

On second thought, it strikes me after looking at several issues related to number inputs, validation, and presentation (thousands separators, l10n/i18n) that none of these are particularly unique problems to Enketo except for what to do with invalid model values. Otherwise these are all problems others have solved, probably many times over.

I wonder if we might reconsider whether it makes sense to reinvent this set of wheels, when there are probably high quality solutions available for reuse. It was looking at #858 which prompted this thought, but I'm leaving this comment here because it calls into question much of my earlier comment. Both this issue and #858 would almost definitely be solvable with an existing dedicated library which is itself well tested.

[lognaturel]

I think this is a complete summary of issues from the thread:

• Firefox: arbitrary combinations of , - . are allowed but saved as blank in the model
• Firefox English (US) locale: , is allowed as a decimal separator but saved as blank in the model
• Chrome: - is allowed anywhere but saved as blank in the model
• Blank values are submitted even for required fields

with behavior consistent with Chrome

I believe there would additionally have to be something for -

Using the pattern attribute

That would require using text rather than number, right? And also could that be locale-aware?

there are probably high quality solutions available for reuse

I'm surprised I'm not quickly finding something all that promising, hopefully you'll have better luck. I see AutoNumeric which has some nice things about it but all the separators need to be hard-coded (not locale-aware). And then this has some good ideas, I think.

[lognaturel]

Also related to https://github.com/kobotoolbox/enketo-express/issues/684 which has good values to consider and add tests for.

- affects integers as well. You can type in 8-3-4-3-3-3 and get a blank value in the submission.