Problem/Motivation

Calling EntityEmbedDisplayManager::getDefinitionOptionsForEntityType() on PHP 8.1 triggers a deprecation: 

Deprecated function: strpos(): Passing null to parameter #1 ($haystack) of type string is deprecated
Drupal\Component\Plugin\PluginBase->getDerivativeId()() (Line: 83)

This happens because in FieldFormatterEntityEmbedDisplayBase's __contruct method EntityEmbedDisplayBase::setConfiguration() is called, and that method call then invokes FieldFormatterEntityEmbedDisplayBase::defaultConfiguration() before the plugin ID was set.

Steps to reproduce

E.g. execute Entity Embed tests on PHP 8.1.

Proposed resolution

In the constructor, set the the plugin ID manually before setConfiguration is invoked.

Remaining tasks

TBD.

User interface changes

Nothing!

API changes

Nothing!

Data model changes

Nothing!

CommentFileSizeAuthor
#5 interdiff-3260833-2-5.txt890 byteshuzooka
#5 entity_embed-php81-3260833-5.patch1.84 KBhuzooka
#2 entity_embed-php81-3260833-2--fix.patch999 byteshuzooka
#2 entity_embed-php81-3260833-2--fail.patch966 byteshuzooka

Issue fork entity_embed-3260833

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

huzooka created an issue. See original summary.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Assigned: huzooka » Unassigned
Status: Active » Needs review
StatusFileSize
new entity_embed-php81-3260833-2--fail.patch966 bytes
new entity_embed-php81-3260833-2--fix.patch999 bytes

The last submitted patch, 2: entity_embed-php81-3260833-2--fail.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Status: Needs review » Needs work

The last submitted patch, 2: entity_embed-php81-3260833-2--fix.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Status: Needs work » Needs review
StatusFileSize
new entity_embed-php81-3260833-5.patch1.84 KB
new interdiff-3260833-2-5.txt890 bytes

Let's use a test png with valid profile!

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Issue summary: View changes

We're expecting the same test failure what HEAD has at https://www.drupal.org/node/2239435/qa

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Title: Construction of EntityEmbedDisplay plugin instances triggers strpos deprecation on PHP 8.1 » Construction of EntityEmbedDisplay plugin instances trigger strpos deprecation on PHP 8.1

Status: Needs review » Needs work

The last submitted patch, 5: entity_embed-php81-3260833-5.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented
Status: Needs work » Needs review

Coding standard violation is a preexisting thing, test failure is the same as on HEAD.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Reviewed & tested by the community 
+++ b/tests/src/Functional/EntityEmbedDisplayManagerTest.php
@@ -82,10 +82,7 @@ class EntityEmbedDisplayManagerTest extends BrowserTestBase {
-    $entity_embed_path = $this->container->get('module_handler')
-      ->getModule('entity_embed')
-      ->getPath();
-    \Drupal::service('file_system')->copy($entity_embed_path . '/js/plugins/drupalentity/entity.png', 'public://example1.png');
+    \Drupal::service('file_system')->copy('core/tests/fixtures/files/image-1.png', 'public://example1.png');

Ohhhh — clever way to avoid file path futzing: by just using a file used by core! :D

Dave Reid made their first commit to this issue’s fork.

Dave Reid opened merge request !6

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

I've gone through fixing the branch tests to all passing now. I think we can also avoid needing to use __construct() for all these plugins because it's better to just override create(). We can also just add a formatterPluginManager() method that will always return the service even if it hasn't been injected yet. Going to test against PHP 8.1 to see if that resolves the failures.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
commented

@Dave Reid, thank you for pushing this forward!

  • Dave Reid committed 3184cd0 on 8.x-1.x 
    Issue #3260833 by huzooka, Dave Reid: Fixed...
dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Reviewed & tested by the community » Fixed

Merged the above, confirmed passing tests on PHP 8.1 into 8.x-1.x. Thanks!

Status: Fixed » Closed (fixed)

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