Problem/Motivation

template_preprocess_views_view_rss() has the following line:

  $variables['channel_elements'] = \Drupal::service('renderer')->render($style->channel_elements);

The channel elements are then printed in views-view-rss.html.twig:

<rss version="2.0" xml:base="{{ link }}"{{ namespaces }}>
  <channel>
    <title>{{ title }}</title>
    <link>{{ link }}</link>
    <description>{{ description }}</description>
    <language>{{ langcode }}</language>
    {{ channel_elements }}
    {{ items }}
  </channel>
</rss>

By forcing the {{ channel_elements }} to render early, its contents can no longer be changed by other modules. The Views RSS module for example extends the channel elements with more elements from the RSS spec: http://www.rssboard.org/rss-profile

This module now has to re-render the channel elements with the updated channel elements.

Proposed resolution

Remove early rendering of RSS channel elements in template_preprocess_views_view_rss() so other modules can add channel elements without having to re-render.

Remaining tasks

  1. Write a patch
  2. Review
  3. Commit

User interface changes

None.

API changes

{{ channel_elements }} in views-view-rss.html.twig now is a renderable array instead of rendered markup.

Data model changes

None.

Comments

idebr created an issue. See original summary.

idebr’s picture

Issue summary: View changes
idebr’s picture

Status: Active » Needs review
StatusFileSize
new2.27 KB
new3.03 KB

Attached patch removes early rendering of RSS channel elements in template_preprocess_views_view_rss() so other modules can add channel elements without having to re-render. It also provides a new views_test_rss module that provides hooks to test Views RSS output and adds test coverage for the {{ channel_elements }}.

The last submitted patch, 3: 3070978-2-test-only.patch, failed testing. View results

lendude’s picture

Status: Needs review » Reviewed & tested by the community

This change makes sense and nice to see test coverage added for this.

Dug a little into the 'why' of this, because the added early rendering makes little sense to me, so this may have been done for a reason. But the only explanation I could find was 'legacy code'. This is old code that hasn't been touched since Views was added to core other then updating some function calls to service calls.

idebr’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new2.36 KB
new3.12 KB

The last submitted patch, 6: 3070978-6-test-only.patch, failed testing. View results

lendude’s picture

Status: Needs review » Reviewed & tested by the community

Reroll looks good, back to RTBC

The last submitted patch, 6: 3070978-6-test-only.patch, failed testing. View results

The last submitted patch, 6: 3070978-6-test-only.patch, failed testing. View results

The last submitted patch, 6: 3070978-6-test-only.patch, failed testing. View results

The last submitted patch, 6: 3070978-6-test-only.patch, failed testing. View results

The last submitted patch, 6: 3070978-6-test-only.patch, failed testing. View results

idebr’s picture

StatusFileSize
new3.12 KB

Reupload of #6 in an effort to reduce testbot noise.

wim leers’s picture

Nice simplification :) And one fewer call to \Drupal::service('renderer')->render() too! 🥳

larowlan’s picture

Added a change record for those who're doing something like str_replace or re-rendering https://www.drupal.org/node/3074409

larowlan’s picture

Status: Reviewed & tested by the community » Fixed

Committed 5938132 and pushed to 8.8.x. Thanks!

Published the change record

  • larowlan committed 5938132 on 8.8.x
    Issue #3070978 by idebr: Remove early rendering of RSS channel elements...

Status: Fixed » Closed (fixed)

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