Fix warning when RSS feed includes markup with an empty srcset attribute

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request → as a guide.

As this is a bug it will need a test case to show the issue.

Also an issue summary update for steps to reproduce, proposed solution, remaining tasks, etc. Should follow the default template.

Hi i experience the same issue on core 9.5.9 and it generates large error messages in session table which can generate exceding max_allowed_packet error when querying session table on common mysql config.

Updated issue summary.

Test-only patch; should fail.

Fix + test; should pass.

The last submitted patch, 10: 3195583-test-only.patch, failed testing. View results →

test seems to show the issue pretty clearly.

Followed the steps from the issue summary and definitely see the issue

Patch #11 does solve the issue.

I am doing triage on the core RTBC queue → .

The issue summary and proposed resolution are clear and complete. Thank you, that really helps the reviewers and committers. I read the comments this still needs a code review. Setting to Needs review.

I tested this on Drupal 11.x, using the steps in the issue summary and reproduced that problem.
I then took a look at the patch.

+++ b/core/lib/Drupal/Component/Utility/Html.php
@@ -471,8 +471,11 @@ public static function transformRootRelativeUrlsToAbsolute($html, $scheme_and_ho
+          if (isset($image_candidate_string[0]) && isset($image_candidate_string[1])) {

Since this is really just checking for an empty string this could be much simpler.

        $image_candidate_strings = array_filter(array_map('trim', $image_candidate_strings));
        foreach ($image_candidate_strings as $key => $image_candidate_string) {
          if ($image_candidate_string[0] === '/' && $image_candidate_string[1] !== '/') {
            $image_candidate_strings[$key] = $scheme_and_host . $image_candidate_string;
          }

Now, setting to Needs work.

This also needs a title to say what is being fixed here.

Rerolled the patch against 11.x and updated it as suggested in #15.

Per #16

Updated issue title, as requested.

Thanks!

Patch does not apply. Thanks!

Re-rolled for current 11.x-dev.

Patch failed to apply.

Let me work on it.

Updated the patch as per latest 11.x version.
Please review.

#25 didn't apply either.

Thanks. #26 has an unnecessary variable assignment which wasn't present in #17. I've attached a modified patch that removes that line (and an interdiff).

(Unintentionally changed issue status. Sorry for the noise.)

lets see what the tests say but that line I believe was new to the existing code. But if not needed then good cleanup.

Both patches failed. Investigating…

Revised test data since the transformation now removes empty attribute values.

The Needs Review Queue Bot → tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request → . Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

Think this was a false positive by the bot.

Nope needs an MR now and patches hidden.

Created https://git.drupalcode.org/project/drupal/-/merge_requests/5355.

I triggered the test-only job for this on GitLab CI. Results:

There was 1 error:
1) Drupal\Tests\Component\Utility\HtmlTest::testTransformRootRelativeUrlsToAbsolute with data set "weepwas0, empty srcset" ('<weepwas0 srcset>empty test</weepwas0>', 'http://example.com', false)
Uninitialized string offset 0
/builds/issue/drupal-3195583/core/lib/Drupal/Component/Utility/Html.php:490
/builds/issue/drupal-3195583/core/tests/Drupal/Tests/Component/Utility/HtmlTest.php:345
/builds/issue/drupal-3195583/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
/builds/issue/drupal-3195583/vendor/phpunit/phpunit/src/Framework/TestSuite.php:684
/builds/issue/drupal-3195583/vendor/phpunit/phpunit/src/Framework/TestSuite.php:684
/builds/issue/drupal-3195583/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:651
/builds/issue/drupal-3195583/vendor/phpunit/phpunit/src/TextUI/Command.php:144
/builds/issue/drupal-3195583/vendor/phpunit/phpunit/src/TextUI/Command.php:97

Saving issue credits.

Committed to 11.x and 10.2.x, thanks!

This is also eligible for backport to 10.1.x as a non-disruptive bugfix; however, it does not cherry-pick cleanly (nor does #31 apply to 10.1.x). Marking "Needs work" for a 10.1.x backport of the final version of the patch to be created. Thanks!

xjm → closed merge request !5355

Better status.

Thanks, @xjm. Here's an MR for 10.1.x: https://git.drupalcode.org/project/drupal/-/merge_requests/5362

Committed to 10.1.x. Thanks @smokris!

xjm → closed merge request !5362

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