Problem/Motivation

See #2567743-16: Add protocol filtering to Attribute, which use modifies <drupal-render-placeholder callback=… arguments=… token=…> to <drupal-render-placeholder data-callback=… arguments=… token=…>.

The problem with that can be found in the accompanying comment:

    // Generate placeholder markup. Note that the only requirement is that this
    // is unique markup that isn't easily guessable. The #lazy_builder callback
    // and its arguments are put in the placeholder markup solely to simplify
    // debugging.

Prefixing it with data- makes it less developer-friendly.

Proposed resolution

This could (and IMO should) just not use Attribute, and do a SafeString::create(). This is internal in the Renderer anyway.

Remaining tasks

Review.

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#2 2569371-2.patch1.91 KBWim Leers
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers created an issue. See original summary.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Active » Needs review
FileSize
2569371-2.patch1.91 KB
catch’s picture

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

Yep, that's a better change, no need to run that through Attributes the whole point is this is hardcoded.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

Nice - and it'll make it easier to convert SafeMarkup::format() to return an object like t() too.

Committed 4a7ffc9 and pushed to 8.0.x. Thanks!

  • alexpott committed 4a7ffc9 on 8.0.x 
    Issue #2569371 by Wim Leers: Update Renderer::createPlaceholder() to not...
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Tag1 Consulting for Drupal Association commented 
+++ b/core/lib/Drupal/Core/Render/Renderer.php
@@ -703,17 +703,16 @@ protected function createPlaceholder(array $element) {
+    $arguments = UrlHelper::buildQuery($placeholder_render_array['#lazy_builder'][1]);

Can we trust those arguments?

Status: Fixed » Closed (fixed)

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

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

Looks like the new generated placeholders break when parsed as XML: #2593481: PlaceholderGenerator::createPlaceholder() generates invalid markup; causes placeholders to not be replaced if processed by DOMDocument