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.
When adding a content-type with an integer field, I got the following:
- Notice: Undefined index: scale in number_field_formatter_settings_summary() (line 265 of modules/field/modules/number/number.module).
- Notice: Undefined index: thousand_separator in number_field_formatter_settings_summary() (line 265 of modules/field/modules/number/number.module).
- Notice: Undefined index: prefix_suffix in number_field_formatter_settings_summary() (line 266 of modules/field/modules/number/number.module).
- Notice: Undefined index: thousand_separator in number_field_formatter_settings_form() (line 229 of modules/field/modules/number/number.module).
- Notice: Undefined index: prefix_suffix in number_field_formatter_settings_form() (line 251 of modules/field/modules/number/number.module).
- Notice: Undefined index: scale in number_field_formatter_settings_summary() (line 265 of modules/field/modules/number/number.module).
- Notice: Undefined index: decimal_separator in number_field_formatter_settings_summary() (line 265 of modules/field/modules/number/number.module).
- Notice: Undefined index: scale in number_field_formatter_settings_summary() (line 265 of modules/field/modules/number/number.module).
- Notice: Undefined index: decimal_separator in number_field_formatter_settings_summary() (line 265 of modules/field/modules/number/number.module).
- Notice: Undefined index: thousand_separator in number_field_formatter_settings_summary() (line 265 of modules/field/modules/number/number.module).
- Notice: Undefined index: prefix_suffix in number_field_formatter_settings_summary() (line 266 of modules/field/modules/number/number.module).
The attached patch fixes the problem as follows:
- Adds the following function:
/** * Default number settings * * @param string $number_type * The type of this number field, such as * 'number_integer' or 'number_decimal'. * * @return * An array of default field settings. * * @todo Should be locale-dependent. */ function number_settings_defaults($number_type) { static $common_defaults = array( 'thousand_separator' => ' ', 'decimal_separator' => '.', 'prefix_suffix' => TRUE, ); return $common_defaults + ($number_type == 'number_integer') ? array('scale' => 0 ) : array('scale' => 2, 'precision' => 10); }
- Ensure that the defaults are added where appropriate.
Comments
Comment #2
pillarsdotnet CreditAttribution: pillarsdotnet commentedCorrected...
Comment #4
pillarsdotnet CreditAttribution: pillarsdotnet commented#2: number.module.patch queued for re-testing.
Comment #6
pillarsdotnet CreditAttribution: pillarsdotnet commentedIf at first you don't succeed...
Comment #7
pillarsdotnet CreditAttribution: pillarsdotnet commentedMissed one; corrected.
Comment #8
klausiI cannot reproduce this when I add a content type and an integer field. On what page do the notices occur?
Comment #9
pillarsdotnet CreditAttribution: pillarsdotnet commentedHappened when I add a content type an an integer field. Will try to reproduce on minimal install.
Comment #10
pillarsdotnet CreditAttribution: pillarsdotnet commentedDoesn't happen on minimal install with only "Field UI" and "Number" modules enabled.
Perhaps this is an entity bug? Still testing...
Comment #11
pillarsdotnet CreditAttribution: pillarsdotnet commentedOkay, I give up. After backing out the above patch, I can't reproduce either.
Will reopen if I find a reproducible test case.
Comment #12
VeeLin CreditAttribution: VeeLin commentedIf it helps I got this error in the following situation with :
1. go to Manage Display and change from Default to Unformatted under Format.
2. save
3. change back to default and the error disappearers.
Comment #13
Jarode CreditAttribution: Jarode commentedComment #14
yched CreditAttribution: yched commentedThe 'Unformatted' formatter doesn't have any settings and therefore shouldn't return any 'settings summary'.
The correct fix is to wrap a
if ($display['type'] == 'number_decimal' || $display['type'] == 'number_float') {
check around the following lines in number_field_formatter_settings_summary() :Additionally, the same
if ($display['type'] == ...
check already exists in number_field_formatter_settings_form() but it needs to be moved just above the$options = array(
line.I'm not where I can roll a patch right now. Anyone ?
Comment #15
yched CreditAttribution: yched commentedComment #16
uiexp CreditAttribution: uiexp commented#7: number.module.patch queued for re-testing.
Comment #17
pillarsdotnet CreditAttribution: pillarsdotnet commented#14 needs to be done for 8.x first, then backported.
Comment #18
Anonymous (not verified) CreditAttribution: Anonymous commentedI tried making the changes that yched suggested in #14, but that didn't solve the problem for me. However, the patch from #7 did solve the problem.
Maybe these are two separate issues that display the same behavior? For me, it is coming up when I use Views to display an integer.
Comment #19
amfis CreditAttribution: amfis commentedFor D7 that helped:
from (line: 264, number.module):
to:
Comment #20
Rhodungeon CreditAttribution: Rhodungeon commented#19 woked perfectly in D7
Comment #21
pillarsdotnet CreditAttribution: pillarsdotnet commentedAdded ysched's suggestion in #14 to patch in #7, cleaned up slightly and re-rolled.
Comment #22
pillarsdotnet CreditAttribution: pillarsdotnet commentedIncidentally, if anyone can point me to the discussion that led to the current decision to make the thousand_separator default to a space, I'd love to read it.
EDIT: Opened #1275978: The thousand_separator for numeric fields should default to '' (nothing) instead of ' ' (space).
Comment #23
yched CreditAttribution: yched commented- I don't favor the introduction of the number_settings_defaults() helper function.
- This function happily mixes field settings ('scale'...) and formatter settings ('thousands separator'), and adds settings that do not make sense ('scale' for integer fields ?)
I think #17 outlines the correct fixes, which are quite simple.
Comment #24
pillarsdotnet CreditAttribution: pillarsdotnet commented@ysched: Sure, whatever; you're obviously smarter than I am, but @linclark said he tried your fix and it didn't work.
Also, the change you suggest makes it impossible to specify a
thousand_separator
for integer numbers. Is that the intent?Conversely, if we disallow
thousand_separator
for integer and unformatted numbers, shouldn't we also disallowprefix_suffix
as well?Comment #25
yched CreditAttribution: yched commented@pillardotsnet : sorry for #23 sounding a bit harsh, I typed it quickly from my phone browser. Thanks for keeping up pounding on this issue.
Actually there's another error in number_field_formatter_settings_[form|summary]() : they reason on formatter being either 'number_decimal' or 'number_float' - and the latter doesn't exist, it should be 'number_integer' instead.
Attached patch is the same as #24, plus this additional fix.
Those fixes are the correct ones for me, and with those applied I haven't been able to reproduce the errors/warnings @linclark mentions in #18 while displaying an integer field in Views - nor do I see where they would come from. So, until @linclark or someone else provides more info on those, I'll stick to that approach.
I cannot RTBC this myself anymore, though :-/
Comment #26
pillarsdotnet CreditAttribution: pillarsdotnet commentedSo in your proposed fix,
$element['decimal_separator']
and$element['scale']
are only defined for type'number_decimal'
:But they are referenced for both types
'number_decimal'
and'number_integer'
:Apologies for my ignorance, but please explain to me how this will not result in an undefined index error.
Also,
After applying your patch:
If
'number_float'
doesn't exist, there is certainly a lot of cruft that needs cleaning up.Comment #27
pillarsdotnet CreditAttribution: pillarsdotnet commentedAlso, since our mutually contradictory patches are passing all tests, whatever fix is decided upon needs a corresponding test added.
Comment #28
yched CreditAttribution: yched commentedNo, 'decimal_separator' and 'scale' settings are defined for the 'number_integer' formatter too (see number_field_formatter_info()), they just have no UI in the form, so they will be present with their default values. Thus, referencing them will generate no warnings, they exist, just not changeable (at least not in the UI). I reckon this is not exactly intuitive, but this allows the formatter_view() and formatter_summary() code to be the same regardless of the field type (integer, decimal, float).
You're mixing field types and formatters. The 'number_float' field type exists, but there is no formatter named 'number_float'. float fields use the 'number_decimal' formatter.
So yes, the patch is correct. Updated patch just adds a comment in number_field_formatter_info() to make that a little clearer.
Comment #29
pillarsdotnet CreditAttribution: pillarsdotnet commentedLooks good; the only remaining task is to roll a test. I'll try to do that.
Comment #30
yched CreditAttribution: yched commentedThe 'formatter settings' UI is not easily testable currently. That's because it is triggered by an image button, and those do not play well with our testing framework.
I guess this could be tested by programmatically creating a integer field and instance with the number_integer formatter, and just displaying the "Manage display" page (the test will pass by just triggering no warnings), but to be honest, I'm not sure a test is worth it.
Comment #31
pillarsdotnet CreditAttribution: pillarsdotnet commentedNevertheless, I'm writing one, and learning a lot in the process.
Comment #32
pillarsdotnet CreditAttribution: pillarsdotnet commentedTest fails without the patch; passes with it.
Comment #33
pillarsdotnet CreditAttribution: pillarsdotnet commented#32: undefined-number-format-1002734-32-test+fix.patch queued for re-testing.
Comment #34
yched CreditAttribution: yched commentedGetting back at this after a couple weeks away from my coding env.
Thanks for the tests ! Just slightly adjusted those (explicitly test setting the formatter to 'number_integer', so that the test does not depend on it being the default formatter).
@pillarsdotet validated the code, I validate the tests (with the minor adjustment above), so I guess this is RTBC.
Comment #35
chx CreditAttribution: chx commentedI guess so too :)
Comment #36
pillarsdotnet CreditAttribution: pillarsdotnet commentedRemoving "needs tests" tag.
Comment #37
webchickCommitted and pushed to 8.x and 7.x. Thanks!
Comment #39
acb CreditAttribution: acb commentedI am still seeing this error flooding watchdog. Has there been a regression somewhere?