HtmlResponseAttachmentsProcessor::processAttachments lists the keys expected in the #attached part of the final render array (i.e. $response->getAttachments()):

https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Render%21...

The list includes library and feed, but not placeholders. However, one of the first actions of this method is to call ::renderPlaceholders!

Given that the Drupal 8 render pipeline docs:

https://www.drupal.org/docs/8/api/render-api/the-drupal-8-render-pipeline

try to explain placeholders as being injected at this point, then I think the method's own docblock should at least mention them, even if it makes clear that they should have been set automatically by earlier stages in the pipeline.

CommentFileSizeAuthor
#12 2849246.patch836 bytescatch

Comments

jp.stacey created an issue. See original summary.

jp.stacey’s picture

jp.stacey commented
Title: HtmlResponseAttachmentsProcessor::processAttachments makes no mention of permitted #attached placeholder » HtmlResponseAttachmentsProcessor::processAttachments makes no mention of permitted #attached placeholders
Issue summary: View changes

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

catch’s picture

catch
he/him
Primary language English
commented
Title: HtmlResponseAttachmentsProcessor::processAttachments makes no mention of permitted #attached placeholders » HtmlResponseAttachmentsProcessor::processAttachments should reference processing placeholders
Category: Bug report » Task
Priority: Normal » Minor
Status: Active » Needs review
Issue tags: +Bug Smash Initiative
StatusFileSize
new 2849246.patch836 bytes

The doc for this method comes from the interface, but ::renderPlaceholders() is on the concrete class, so all we can do is cross-reference. Here's a patch, but reclassifying as a task since the documentation is fine, I'm not even sure about the cross-reference from interface method to protected class method, but can't see another way.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Seems like a simple update. Ran against 10.1 to just be sure.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: 10.1.x-dev » 9.5.x-dev
Status: Reviewed & tested by the community » Fixed

Expanded the wording and added a backslash on commit. Backported to 10.0.x and 9.5.x.

   * Placeholders need to be rendered first in order to have all attachments
   * available for processing. For an example, see
   * \Drupal\Core\Render\HtmlResponseAttachmentsProcessor::renderPlaceholders()

A shame that this took so long to fix. Rest in peace, jp.stacey.

  • longwave committed 2270a643 on 10.0.x 
    Issue #2849246 by catch, jp.stacey: HtmlResponseAttachmentsProcessor::...

  • longwave committed 6cfdef1a on 10.1.x 
    Issue #2849246 by catch, jp.stacey: HtmlResponseAttachmentsProcessor::...

  • longwave committed cb0096a7 on 9.5.x 
    Issue #2849246 by catch, jp.stacey: HtmlResponseAttachmentsProcessor::...
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Thanks! 🤩 I never reviewed this because this wasn't under the big_pipe.module component 😅

Status: Fixed » Closed (fixed)

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