Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
I have a number
field which has two decimal places (for currency). After upgrading to the newest version of 7.x-4.0-beta3, I find that I am no longer allowed to submit a form with 0.00
as a valid value. A simple 0
works just fine, although re-editing the form afterwards reverts the value to 0.00
due to the fact that I set the automatic decimal place setting.
Example.
Form with Registration Fee
with type Number
.
What I expect:
Submitting the form with 0.00 is valid.
What I get instead:
Error message saying Registration Fee field value must format numbers as "12,345.68".
Comment | File | Size | Author |
---|---|---|---|
#13 | webform_number_regex-13.patch | 818 bytes | quicksketch |
#12 | webform_number_regex-12.patch | 824 bytes | quicksketch |
#7 | webform_decimal_validate-2201493.patch | 876 bytes | quicksketch |
Comments
Comment #1
schrauger CreditAttribution: schrauger commentedRemoved the "decimal places" portion, since the bug persists after setting "decimal places" to "automatic". It accepts any number and any decimal, including "0" and "0.01", but it fails to validate on "0.00".
Comment #2
quicksketchThanks for the report. We had another issue with those changes at #2198007: Negative numbers always fail validation. I'll try to fix this problem as soon as I get a chance.
Comment #3
DanChadwick CreditAttribution: DanChadwick commentedThis is a tougher problem that it seems at first thought.
In the code, quicksketch references this solution:
http://stackoverflow.com/questions/5917082/regular-expression-to-match-n...
He has modified it to allow an optional leading minus sign followed by an option space (e.g. '- 100').
However, there are valid-looking numbers that the validation rejects, such as:
12,23,456.10 (valid in hindi, apparently)
0.00
1.20
.12
10.
For this discussion, realize that period means the decimal separator and comma means the thousand separator.
I suggest that we be much more lenient. If the number looks like a valid number, even if weirdly or improperly formatted, accept it. It is much worst to reject a valid number than accept in invalid one.
OPTION 1:
(\-|\+)? ?\d*(,\d+)*(\.\d*)?
Optional - or +
Optional space
0 or more digits
Optional comma and some digits, repeated as many times as desired
Optional period followed by optional digits
OPTION 2:
Remove commas and spaces.
Remove leading + or -, if any
Remove 1 period
Remove digits (or test with ctype_digit)
If you have an empty string, it was a valid number.
This option allows a lot of weirdly formatted numbers, such as '+ 2 234, 4564 . 10', which I think is fine.
OPTION 3:
Remove spaces and commas
If no decimal in string, add one to the end. This prevents is_numeric from using octal, hex, or binary.
Call is_numeric.
This will accept exponential notation, which I think is fine. So +001, 23 .3300 is valid (=123.33).
I vote Option 3. I think it is safer (no extremely tricky regex expressions) and is guaranteed to not reject a valid number.
This leaves the related problem of some HTML5 clients returning numbers with a period, even if the locale uses a comma as the decimal:
#2202905: Decimal numbers with comma as placeholder fail validation in Chrome
Option 2 and 3 would accept the number, but I don't know how to extract the value when you can't tell if 1.234 is really > 1000 or < 2.
Comment #4
DanChadwick CreditAttribution: DanChadwick commentedComment #5
DanChadwick CreditAttribution: DanChadwick commented@quicksketch - Option 3 okay with you?
Comment #6
quicksketchSure, option 3 sounds fine. The method doesn't really matter to me so much as the effectiveness of the result. The regex seemed like a good idea but now in practice it's definitely caused some trouble.
Comment #7
quicksketchFor the time being, amending the current regex was the easiest approach to quick fix this. I'm still up for alternatives, but this fixes all the problems mentioned above. I've committed this to get the immediate problem solved.
Comment #9
cs_shadow CreditAttribution: cs_shadow commentedMarking as fixed since the patch is already committed.
Comment #10
DanChadwick CreditAttribution: DanChadwick commentedThanks quicksketch. I had intended to get back to this sooner.
You change uses this pattern (as far as I can tell -- the regEx is pretty complicated)
This won't accept things that I think most people would consider to be numbers:
-0
01 (like a car number)
007 (what would James say?)
02474 (zip code)
-0.01 (you owe me a penny, bitch)
- 0.01
0,123 (might be used to line up numbers in a column for some reason)
I think trying to make numbers match a formatting pattern is more restrictive than needed.
Comment #11
quicksketchUnfortunately, the "number" HTML5 type will also have many of the same behaviors (stripping out leading zeros). So these numbers already aren't allowed in Chrome, or worse, it could strip them out without the user noticing the change.
That said, I think we should still adjust the code to allow leading zeros. Let's consider switching the HTML number element back to text (or something else) in #2202905: Decimal numbers with comma as placeholder fail validation in Chrome.
Comment #12
quicksketchThis adjustment simplifies the Regex further, since if we add zeros to the first pattern, we no longer need the entire second-half of the pattern. I tested this allows the examples from #3 and #10. I'm still totally open to the alternative approaches described in #3, but fixing the existing code is easier than refactoring it.
Comment #13
quicksketchAnother yet more simple regex, swapping out
\d(?:\d{0,2})
with the equivalent\d{1,3}
.Comment #15
quicksketchI think this is fixed for real this time, but as I said above, I'm fine with switching this approach to something easier if someone wants to take it on.
Comment #16
DanChadwick CreditAttribution: DanChadwick commented@quicksketch. Sorry my patch wasn't more prompt. I was currently working on it and looking at the validation flow-of-control for components, about which I know nothing.
Your regex is much better. Do we care that it doesn't accept a + sign? And do we really care that the commas are in the right spots when the formatting will be discarded and reformatted by the component?
This is what I was testing. I think it's solid.
Comment #17
quicksketchThanks @DanChadwick! That actually looks really good. Although it will allow repeated thousands placeholders, e.g. ",,,,,,,,,,,,,,,,,,,,0.2,,,3,,4" (which would save as 0.234). Is that anything to worry about?
Comment #18
DanChadwick CreditAttribution: DanChadwick commentedYes, it allows any number (pun, get it?) of weirdly-formatted numbers. I did not take that to be a bad thing.
Comment #19
fenstratNeeds porting to 8.x-4.x.
Comment #20
fenstratCommitted and pushed 3914747 to 8.x-4.x. Thanks!
Comment #22
fenstrat#13 needs porting to 8.x-4.x.
Comment #23
fenstratCommitted and pushed 6f9f382 to 8.x-4.x. Thanks!