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:

  1. Adds the following function:
    <?php
    /**
    * 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);
    }
    ?>
  2. Ensure that the defaults are added where appropriate.
Files: 
CommentFileSizeAuthor
#34 undefined-number-format-1002734-34-test+fix.patch6.41 KByched
PASSED: [[SimpleTest]]: [MySQL] 33,323 pass(es).
[ View ]
#32 undefined-number-format-1002734-32-testonly.patch2.29 KBpillarsdotnet
FAILED: [[SimpleTest]]: [MySQL] 32,963 pass(es), 0 fail(s), and 4 exception(es).
[ View ]
#32 undefined-number-format-1002734-32-test+fix.patch7.1 KBpillarsdotnet
PASSED: [[SimpleTest]]: [MySQL] 33,290 pass(es).
[ View ]
#28 undefined-number-format-1002734-28.patch4.36 KByched
PASSED: [[SimpleTest]]: [MySQL] 32,946 pass(es).
[ View ]
#25 undefined-number-format-1002734-25.patch3.47 KByched
PASSED: [[SimpleTest]]: [MySQL] 32,941 pass(es).
[ View ]
#24 undefined-number-format-1002734-24.patch3.12 KBpillarsdotnet
PASSED: [[SimpleTest]]: [MySQL] 32,943 pass(es).
[ View ]
#21 undefined-number-format-1002734-21.patch5.83 KBpillarsdotnet
PASSED: [[SimpleTest]]: [MySQL] 32,957 pass(es).
[ View ]
#13 notice_drupal.png103.13 KBJarode
#7 number.module.patch4.42 KBpillarsdotnet
PASSED: [[SimpleTest]]: [MySQL] 31,879 pass(es).
[ View ]
#6 number.module.patch4.39 KBpillarsdotnet
PASSED: [[SimpleTest]]: [MySQL] 30,416 pass(es).
[ View ]
#2 number.module.patch4.39 KBpillarsdotnet
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
number.module.patch4.39 KBpillarsdotnet
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in number.module.
[ View ]

Comments

Status:Needs review» Needs work

The last submitted patch, number.module.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new4.39 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Corrected...

Status:Needs review» Needs work

The last submitted patch, number.module.patch, failed testing.

Status:Needs work» Needs review

#2: number.module.patch queued for re-testing.

Status:Needs review» Needs work

The last submitted patch, number.module.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new4.39 KB
PASSED: [[SimpleTest]]: [MySQL] 30,416 pass(es).
[ View ]

If at first you don't succeed...

StatusFileSize
new4.42 KB
PASSED: [[SimpleTest]]: [MySQL] 31,879 pass(es).
[ View ]

Missed one; corrected.

I cannot reproduce this when I add a content type and an integer field. On what page do the notices occur?

Happened when I add a content type an an integer field. Will try to reproduce on minimal install.

Doesn't happen on minimal install with only "Field UI" and "Number" modules enabled.

Perhaps this is an entity bug? Still testing...

Status:Needs review» Closed (works as designed)

Okay, I give up. After backing out the above patch, I can't reproduce either.

Will reopen if I find a reproducible test case.

If 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.

StatusFileSize
new103.13 KB

Notice : Undefined index: scale dans number_field_formatter_settings_summary() (ligne 265 dans /var/www/vhosts/myrmecofourmis.com/httpdocs/site/modules/field/modules/number/number.module).
Notice : Undefined index: decimal_separator dans number_field_formatter_settings_summary() (ligne 265 dans /var/www/vhosts/myrmecofourmis.com/httpdocs/site/modules/field/modules/number/number.module).
Notice : Undefined index: thousand_separator dans number_field_formatter_settings_summary() (ligne 265 dans /var/www/vhosts/myrmecofourmis.com/httpdocs/site/modules/field/modules/number/number.module).
Notice : Undefined index: prefix_suffix dans number_field_formatter_settings_summary() (ligne 266 dans /var/www/vhosts/myrmecofourmis.com/httpdocs/site/modules/field/modules/number/number.module).

Status:Needs work» Closed (works as designed)

The '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() :

  $summary[] = number_format(1234.1234567890, $settings['scale'], $settings['decimal_separator'], $settings['thousand_separator']);
  if ($settings['prefix_suffix']) {
    $summary[] = t('Display with prefix and suffix.');
  }

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 ?

Status:Closed (works as designed)» Needs work

Status:Closed (works as designed)» Needs review

#7: number.module.patch queued for re-testing.

Version:7.x-dev» 8.x-dev
Status:Needs review» Needs work
Issue tags:+needs backport to D7

#14 needs to be done for 8.x first, then backported.

I 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.

For D7 that helped:

from (line: 264, number.module):

$summary[] = number_format(1234.1234567890, $settings['scale'], $settings['decimal_separator'], $settings['thousand_separator']);
  if ($settings['prefix_suffix']) {
    $summary[] = t('Display with prefix and suffix.');
  }

to:

  if (isset($settings['scale']) && isset($settings['decimal_separator']) && isset($settings['thousand_separator'])) {
    $summary[] = number_format(1234.1234567890, $settings['scale'], $settings['decimal_separator'], $settings['thousand_separator']);
  }
  if (isset($settings['prefix_suffix'])) {
    $summary[] = t('Display with prefix and suffix.');
  }

#19 woked perfectly in D7

Status:Needs work» Needs review
StatusFileSize
new5.83 KB
PASSED: [[SimpleTest]]: [MySQL] 32,957 pass(es).
[ View ]

Added ysched's suggestion in #14 to patch in #7, cleaned up slightly and re-rolled.

Status:Needs work» Needs review
Issue tags:-Needs tests

Incidentally, 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).

Status:Needs review» Needs work

- 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.

Status:Needs work» Needs review
StatusFileSize
new3.12 KB
PASSED: [[SimpleTest]]: [MySQL] 32,943 pass(es).
[ View ]

@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 disallow prefix_suffix as well?

StatusFileSize
new3.47 KB
PASSED: [[SimpleTest]]: [MySQL] 32,941 pass(es).
[ View ]

@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 :-/

So in your proposed fix, $element['decimal_separator'] and $element['scale'] are only defined for type 'number_decimal':

+    if ($display['type'] == 'number_decimal') {
+      $element['decimal_separator'] = array(
+        '#type' => 'select',
+        '#title' => t('Decimal marker'),
+        '#options' => array('.' => t('Decimal point'), ',' => t('Comma')),
+        '#default_value' => $settings['decimal_separator'],
+      );
+      $element['scale'] = array(
+        '#type' => 'select',
+        '#title' => t('Scale'),
+        '#options' => drupal_map_assoc(range(0, 10)),
+        '#default_value' => $settings['scale'],
+        '#description' => t('The number of digits to the right of the decimal.'),
+      );
+    }

But they are referenced for both types 'number_decimal' and 'number_integer':

+  if ($display['type'] == 'number_decimal' || $display['type'] == 'number_integer') {
+    $summary[] = number_format(1234.1234567890, $settings['scale'], $settings['decimal_separator'], $settings['thousand_separator']);

Apologies for my ignorance, but please explain to me how this will not result in an undefined index error.

Also,

they reason on formatter being either 'number_decimal' or 'number_float' - and the latter doesn't exist, it should be 'number_integer' instead.

After applying your patch:

bobvin@bowie:~/www/8.x$ grep -rn number_float modules/
modules/field/modules/number/number.install:22:    case 'number_float' :
modules/field/modules/number/number.module:41:    'number_float' => array(
modules/field/modules/number/number.module:77:  if ($field['type'] == 'number_decimal' || $field['type'] == 'number_float') {
modules/field/modules/number/number.module:196:      'field types' => array('number_decimal', 'number_float'),
modules/field/modules/number/number.module:206:      'field types' => array('number_integer', 'number_decimal', 'number_float'),
modules/field/modules/number/number.module:316:      'field types' => array('number_integer', 'number_decimal', 'number_float'),
modules/field/modules/number/number.module:327:  if ($field['type'] == 'number_decimal' || $field['type'] == 'number_float') {

If 'number_float' doesn't exist, there is certainly a lot of cruft that needs cleaning up.

Status:Needs review» Needs work
Issue tags:+Needs tests

Also, since our mutually contradictory patches are passing all tests, whatever fix is decided upon needs a corresponding test added.

Issue tags:+Needs tests
StatusFileSize
new4.36 KB
PASSED: [[SimpleTest]]: [MySQL] 32,946 pass(es).
[ View ]

So in your proposed fix, $element['decimal_separator'] and $element['scale'] are only defined for type 'number_decimal':

No, '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).

If 'number_float' doesn't exist, there is certainly a lot of cruft that needs cleaning up.

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.

Looks good; the only remaining task is to roll a test. I'll try to do that.

The '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.

to be honest, I'm not sure a test is worth it.

Nevertheless, I'm writing one, and learning a lot in the process.

StatusFileSize
new7.1 KB
PASSED: [[SimpleTest]]: [MySQL] 33,290 pass(es).
[ View ]
new2.29 KB
FAILED: [[SimpleTest]]: [MySQL] 32,963 pass(es), 0 fail(s), and 4 exception(es).
[ View ]

Test fails without the patch; passes with it.

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new6.41 KB
PASSED: [[SimpleTest]]: [MySQL] 33,323 pass(es).
[ View ]

Getting 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.

I guess so too :)

Issue tags:-Needs tests

Removing "needs tests" tag.

Status:Reviewed & tested by the community» Fixed

Committed and pushed to 8.x and 7.x. Thanks!

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

I am still seeing this error flooding watchdog. Has there been a regression somewhere?