Deprecated: mb_check_encoding() error is breaking media/add/image file upload AJAX.

Created on 11 September 2023, 10 months ago
Updated 6 October 2023, 9 months ago

Problem/Motivation

Deprecated: mb_check_encoding() warning is breaking media/add/image file upload AJAX, showing warnings on the page, but most importantly causing the form to not submit due to the deprecation warnings.

Steps to reproduce

With 8.x-1.11, 8.x-1.x-dev:
Adding a file via /media/add/image I see this error at the top of the screen:

Calling mb_check_encoding() without argument is deprecated in /code/web/modules/contrib/pathauto/src/PathautoState.php on line 182 Deprecated: strlen(): Passing null to parameter #1 ($string) of type string is deprecated in /code/web/modules/contrib/pathauto/src/PathautoState.php on line 183

Then when trying to upload an image, after the file is selected and the thumbnail should appear, instead I see an ajax error in the console:

"
An AJAX HTTP error occurred.
HTTP Result Code: 200
Debugging information follows.
Path: /media/add/image?element_parents=field_media_image/widget/0&ajax_form=1
StatusText: parsererror
ResponseText: Deprecated: mb_check_encoding(): Calling mb_check_encoding() without argument is deprecated in /code/web/modules/contrib/pathauto/src/PathautoState.php on line 182
Deprecated: strlen(): Passing null to parameter #1 ($string) of type string is deprecated in /code/web/modules/contrib/pathauto/src/PathautoState.php on line 183
......
(etc)"

Narrowed this down to PathautoFieldItemList

The call in computeValue() which passes $this->getEntity()->id() to PathautoState::getPathautoStateKey without verification allows for this issue to arise.
In this case, I wasn't expecting this call to be made before the Media entity is even saved.

Proposed resolution

Don't call getPathautoStateKey with a null value

Remaining tasks

1. Determine if PathautoFieldItemList::computeValue() should be getting called within the media add form before submission.
2. Correct the assumption that there is an entity id available in PathautoFieldItemList::computeValue()

🐛 Bug report
Status

Postponed: needs info

Version

1.0

Component

Code

Created by

🇬🇧United Kingdom ChrisDarke London

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @ChrisDarke
  • First commit to issue fork.
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.3 & MySQL 5.7
    last update 9 months ago
    46 pass, 2 fail
  • @shubham_jain opened merge request.
  • Status changed to Needs review 9 months ago
  • First commit to issue fork.
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 5.7
    last update 9 months ago
    48 pass
  • Status changed to Needs work 9 months ago
  • 🇨🇭Switzerland Berdir Switzerland

    mb_check_encoding() doesn't have 3 parameters? This introduces a ton of coding standard issues and should imho be solved differently, as you said, we shouldn't call this function at all if there's no ID yet.

  • Status changed to Postponed: needs info 9 months ago
  • 🇨🇭Switzerland Berdir Switzerland

    isNew() checks were added a year ago, you are likely not using the latest version.

Production build 0.69.0 2024