In the processElement() method (in the Dimensions and Measurement elements), '#field_suffix' isn't always defined, leading to a potential PHP notice when you have a field description:

Notice: Undefined index: #field_suffix in Drupal\physical\Element\Measurement::processElement() (line 139 of modules/contrib/physical/src/Element/Measurement.php).

Steps to reproduce:

  1. Add a physical 'Dimensions' or 'Measurements' field to a node type.
  2. Make sure the field has a description.
  3. When you go the node add/edit form that has the Physical field, the PHP notice appears.

This patch makes sure that '#field_suffix' is set, though not sure if this is the best approach, since you end up with an empty '#field_suffix' in some cases.

CommentFileSizeAuthor
#2 undefined_field_suffix-2940428-2.patch898 bytesbrunodbo

Comments

brunodbo created an issue. See original summary.

brunodbo’s picture

brunodbo
he/him
commented
Status: Active » Needs review
StatusFileSize
new undefined_field_suffix-2940428-2.patch898 bytes
brunodbo’s picture

brunodbo
he/him
commented
Issue summary: View changes
joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

@brunodbo What's the steps to reproduce this notice?

brunodbo’s picture

brunodbo
he/him
commented
Issue summary: View changes

@joelpittet Updated the summary with steps.

  • joelpittet committed 63a2e96 on 8.x-1.x authored by brunodbo 
    Issue #2940428 by brunodbo: Undefined '#field_suffix' causing PHP notice
joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Fixed

Thanks, that helps visualize the problem:) I've committed and pushed to the -dev branch.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

@brunodbo looks like this RTBC patch is going to remove this all together, could you review it as well and I may revert this fix and commit that: #2877891: Help text forcing unit select up

brunodbo’s picture

brunodbo
he/him
commented

Heh, something like the patch in #2877891: Help text forcing unit select up was on my mind to do next :) Indeed, it solves this issue and some others, so let's go with that.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Fixed » Closed (duplicate)
bojanz’s picture

bojanz commented
Status: Closed (duplicate) » Needs review

It's bad practice to revert bug fixes just because another issue is going to refactor the code.

  • bojanz committed 4131b23 on 8.x-1.x 
    Issue #2940428 by brunodbo, joelpittet: Undefined '#field_suffix'...
bojanz’s picture

bojanz commented
Status: Needs review » Fixed

Committed again. This gets rid of the notice while the other issue becomes ready.

Thanks, brunodbo.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

It's bad practice to revert bug fixes just because another issue is going to refactor the code.

I've never heard that before. I would think it's more nuanced than a blanket rule. There were more people RTBCing the other patch and I checked with the author of this patch before reverting.

Status: Fixed » Closed (fixed)

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