Problem/Motivation

Display suite is not compatible where other code is using service decoration of Renderer, which is sometimes necessary to override some functions.

Steps to reproduce

Create a service decorator of RendererInterface, then go to manage display. See WSOD/exception.

Proposed resolution

Replace Renderer with RendererInterface in:

  • \Drupal\ds\Plugin\DsField\TokenBase
  • \Drupal\ds\Plugin\DsField\Node\NodeSubmittedBy

Remaining tasks

Implement.

User interface changes

None

API changes

None

Data model changes

None

Issue fork ds-3570559

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

dpi created an issue. See original summary.

dpi’s picture

dpi
Location Perth, Australia
commented

Tagged with 5.x but I'd appreciate if this was also applied to 3.x.

dpi’s picture

dpi
Location Perth, Australia
commented
Issue summary: View changes
dpi’s picture

dpi
Location Perth, Australia
commented
Issue summary: View changes
swentel’s picture

swentel commented
Version: 5.0.x-dev » 8.x-3.x-dev

The 5.x brach is obsolete, can be fixed in 8.x-3.x

ben280398 made their first commit to this issue’s fork.

ben280398 opened merge request !60

ben280398’s picture

ben280398 commented

Identified the files needing changes per the issue description:

src/Plugin/DsField/TokenBase.php
src/Plugin/DsField/Node/NodeSubmittedBy.php

Changes made:
Replaced concrete Renderer class with RendererInterface in both files

Merge Request is linked here for ease of tracking:
https://git.drupalcode.org/project/ds/-/merge_requests/60

dpi’s picture

dpi
Location Perth, Australia
commented
Status: Active » Reviewed & tested by the community

Changes look right.

ben280398’s picture

ben280398 commented

Thanks Dpi, Could you merge it in please?

swentel’s picture

swentel commented
Status: Reviewed & tested by the community » Needs work

There's also still a call using \Drupal::service('renderer')->render() in DynamicTwigField - maybe we can replace that as Tokenbase has the renderer property. DsExtrasController also has one, let's change that too while we're at it. The one in DsRegionBlock is ok, since the plugin is deprecated anyway.

@dpi : the current patch fixes your WSOD on the manage display screen already? (I haven't added a decorator yet, but if it's fine for you, I'll commit when the two above are done as well.

dpi’s picture

dpi
Location Perth, Australia
commented

the current patch fixes your WSOD on the manage display screen already? (

yeah ive tried the MR out as a patch, I noted the one line I needed to change on my unpatched version was also changed here.

As far as refactoring for dependency injection, I intentionally left that out of the issue summary as maintainers dont have consistent views on this. For example in my projects, I would forbid it.

Leaving as NW for DI, but I personally consider this RTBC.

swentel’s picture

swentel commented
Status: Needs work » Reviewed & tested by the community

Right, makes sense, I'll merge it in, we can always create a follow when needed.

swentel’s picture

swentel commented
Status: Reviewed & tested by the community » Fixed

Merged and tagged a new release

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

swentel’s picture

swentel commented
Status: Fixed » Closed (fixed)