Problem/Motivation

When using focal point in a subform (e.g. an image field with focal point enabled in a paragraphs subform), validation fails with the following message, which makes the whole form unsaveable:

Notice: Undefined index: focal_point in Drupal\focal_point\Plugin\Field\FieldWidget\FocalPointImageWidget::validateFocalPoint() (line 120 of modules/contrib/focal_point/src/Plugin/Field/FieldWidget/FocalPointImageWidget.php).

This happens due to the way, the current focal point value is determined from the form state object:

$field_name = array_shift($element['#parents']);
$field_values = $form_state->getValue($field_name);

Proposed resolution

Provide a patch that uses $element['#value'] directly as other core widget plugins do it.

Remaining tasks

  • Create patch
  • Check why tests are failing now

Steps to reproduce

  • Install paragraphs module
  • Install latest focal_point dev-release module
  • Create paragraphs type containing an image field
  • Add a paragraphs-enabled field (entity referecence revision) field to a content type
  • At "Manage for display" set the edit mode of the paragraphs field to "Preview"
  • Try to create a content with the paragraphs type containing the image field
CommentFileSizeAuthor
#13 focal_point_validation-2764403-13.patch2.97 KBbleen
#13 interdiff.txt1.55 KBbleen
#4 interdiff-2764403-2-3.txt869 byteshctom
#4 focal_point_validation-2764403-3.patch1.42 KBhctom
#2 focal_point_validation-2764403-2.patch1.39 KBhctom
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hctom created an issue. See original summary.

hctom’s picture

hctom
Primary language German
CreditAttribution: hctom at undpaul commented
Status: Active » Needs review
FileSize
focal_point_validation-2764403-2.patch1.39 KB

Attached is the proposed path. This patch also fixes an issue with the error message thrown on validation errors:

User error: Invalid placeholder (!title) in string: The !title field should be in the form "leftoffset,topoffset" where offsets are in percents. Ex: 25,75. inDrupal\Component\Render\FormattableMarkup::placeholderFormat()

I'd appreciate your feedback ;)

Status: Needs review » Needs work

The last submitted patch, 2: focal_point_validation-2764403-2.patch, failed testing.

hctom’s picture

hctom
Primary language German
CreditAttribution: hctom at undpaul commented
Status: Needs work » Needs review
FileSize
focal_point_validation-2764403-3.patch1.42 KB
interdiff-2764403-2-3.txt869 bytes

Oops, I missed the case, where the focal point field may be completely empty. So here is a follow up patch covering that, too.

Status: Needs review » Needs work

The last submitted patch, 4: focal_point_validation-2764403-3.patch, failed testing.

hctom’s picture

hctom
Primary language German
CreditAttribution: hctom at undpaul commented

@bleen18: There is some strange stuff going on in the tests... can you please have a look, why those fail now?

Thanx in advance

bleen’s picture

bleen CreditAttribution: bleen at NBCUniversal commented

@hctom ... I have no problem with the change to the error message itself, but based on the test failures it seems that the using $element['#value'] cannot be used here because (like "alt" and "title") the focal point value is attached to the image itself and is not a value unto itself.

Can you give some more detailed steps to reproduce this issue?

hctom’s picture

hctom
Primary language German
CreditAttribution: hctom at undpaul commented
Issue summary: View changes
hctom’s picture

hctom
Primary language German
CreditAttribution: hctom at undpaul commented
bleen’s picture

bleen CreditAttribution: bleen at NBCUniversal commented

...one way to fix this might be to add the validation to the image field itself instead of the form

hctom’s picture

hctom
Primary language German
CreditAttribution: hctom at undpaul commented
Issue summary: View changes

Just added some info how to reproduce this

hctom’s picture

hctom
Primary language German
CreditAttribution: hctom at undpaul commented

I think I found something in the tests ;)

The FocalPointImageWidget::validateFocalPoint() element validation handler is bound to the focal_point field in the widget:

 $element['focal_point'] = array(
      '#type' => 'textfield',
      '#title' => 'Focal point',
      '#description' => new TranslatableMarkup('Specify the focus of this image in the form "leftoffset,topoffset" where offsets are in percents. Ex: 25,75'),
      '#default_value' => isset($item['focal_point']) ? $item['focal_point'] : \Drupal::config('focal_point.settings')->get('default_value'),
      '#element_validate' => array('\Drupal\focal_point\Plugin\Field\FieldWidget\FocalPointImageWidget::validateFocalPoint'),
      '#attributes' => array(
        'class' => array('focal-point', $element_selector),
        'data-selector' => $element_selector,
        'data-field-name' => $element['#field_name'],
      ),
      '#attached' => array(
        'library' => array('focal_point/drupal.focal_point'),
      ),
    );

But in the tests, you call this method with the image test element as parameter but not with the actual focal_point element:

$FocalPointImageWidget::validateFocalPoint($this->testElement, $this->testFormState);

So the parameter passed in the test varies from the parameter Drupal would pass in, when running form validation, doesn't it?

bleen’s picture

bleen CreditAttribution: bleen at NBCUniversal commented
Status: Needs work » Needs review
FileSize
interdiff.txt1.55 KB
focal_point_validation-2764403-13.patch2.97 KB

Ok ... I had a few mins to take a look. The patch looks good but there was a slightly different issue with the unit tests than you suspected. The primary issue was that the test presumed that the test value had to be in an array that looked like the #parents array. Since thats no longer relevant I just moved it around in the test.

This should do it.

jastraat’s picture

jastraat CreditAttribution: jastraat at Technivant commented
Status: Needs review » Reviewed & tested by the community

The patch in #13 addressed the error that occurred when editing a node containing a field using the Inline Entity Form module for entity reference entry when the entity contained an image field configured with a focal point style.

Error addressed:
Notice: Undefined index: focal_point in Drupal\focal_point\Plugin\Field\FieldWidget\FocalPointImageWidget::validateFocalPoint()

bleen’s picture

bleen CreditAttribution: bleen at NBCUniversal commented
Title: Focal point validation fails when used in subform » Focal point validation fails when it's not a top-level form element

  • bleen committed 56977e8 on 8.x-1.x 
    Issue #2764403 by hctom, bleen: Focal point validation fails when it's...
bleen’s picture

bleen CreditAttribution: bleen at NBCUniversal commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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