Problem/Motivation

In #3546376: Use the 'yield' option instead of output buffering for twig rendering to support async rendering I advocated for converting MarkupInterface objects into Twig\Markup objects in order to retain the safeness indications. However after reading the upstream code - see https://github.com/twigphp/Twig/blob/3.x/src/Runtime/EscaperRuntime.php#... and specifically the way it cast objects with classes or interfaces in \Twig\Runtime\EscaperRuntime::$safeClasses to string I feel I made the wrong call.

Proposed resolution

Don't use Twig\Markup objects - just cast to string an be done.

Remaining tasks

Update a change record, https://www.drupal.org/node/3546663

User interface changes

None

Introduced terminology

None

API changes

None

Data model changes

None

Release notes snippet

N/a

Issue fork drupal-3547965

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

alexpott created an issue. See original summary.

alexpott opened merge request !13292

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
Related issues: +#3546376: Use the 'yield' option instead of output buffering for twig rendering to support async rendering
nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs review » Reviewed & tested by the community

The new tests are still passing and it makes sense based on upstream code,

  • catch committed 632dc809 on 11.x 
    Issue #3547965 by alexpott: Don't create Twig\Markup objects...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x, thanks!

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

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

Maintainers, please credit people who helped resolve this issue.

catch closed merge request !13292

quietone’s picture

quietone commented
Issue summary: View changes
Status: Fixed » Needs work

I don't see that the change record has been updated.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Fixed

CR is correct afaict, not sure if it was updated since, or possibly not before the last minute change in the previous issue.

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

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

Maintainers, please credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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