contextual_pre_render_placeholder simply outputs raw HTML instead of using the new attribute system. This results in unescaped ampersands in HTML output.

Wrong:

<div data-contextual-id="views_ui:admin/structure/views/view:frontpage:location=page&name=frontpage&display_id=page_1"></div>

I will upload the patch as soon as I get an issue number.

CommentFileSizeAuthor
#10 contextual_pre_render_placeholder_attributes-2003684-10.patch5.47 KBWim Leers
#9 2003684-9.contextual_pre_render_placeholder_attributes.patch5.01 KBrszrama
#5 2003684-5.contextual_pre_render_placeholder_attributes.patch5.01 KBrszrama
#3 2003684.contextual_pre_render_placeholder_attributes.patch776 bytesrszrama
#1 escape-ids-2003684-1.patch922 bytesnicholaspaun
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

nicholaspaun’s picture

nicholaspaun CreditAttribution: nicholaspaun commented
Status: Active » Needs review
FileSize
escape-ids-2003684-1.patch922 bytes

Status: Needs review » Needs work

The last submitted patch, escape-ids-2003684-1.patch, failed testing.

rszrama’s picture

rszrama CreditAttribution: rszrama commented
Status: Needs work » Needs review
FileSize
2003684.contextual_pre_render_placeholder_attributes.patch776 bytes

Re-rolled patch attached with coding style in line with other uses of Attribute. The new HTML looks like so:

<div data-contextual-id="views_ui:admin/structure/views/view:frontpage:location=page&amp;name=frontpage&amp;display_id=page_1"></div>

Status: Needs review » Needs work

The last submitted patch, 2003684.contextual_pre_render_placeholder_attributes.patch, failed testing.

rszrama’s picture

rszrama CreditAttribution: rszrama commented
Status: Needs work » Needs review
FileSize
2003684-5.contextual_pre_render_placeholder_attributes.patch5.01 KB

Updated the "use" to not have a preceding \ and fixed the failing tests to also make use of Attribute.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented

Ohhh, great find! I'm the one who introduced this, so I'll review this until it's committed. Let's first see if this still passes.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented

#5: 2003684-5.contextual_pre_render_placeholder_attributes.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 2003684-5.contextual_pre_render_placeholder_attributes.patch, failed testing.

rszrama’s picture

rszrama CreditAttribution: rszrama commented
Status: Needs work » Needs review
FileSize
2003684-9.contextual_pre_render_placeholder_attributes.patch5.01 KB

Attempting a quick re-roll.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented
Issue summary: View changes
Issue tags: +Spark, +sprint
FileSize
contextual_pre_render_placeholder_attributes-2003684-10.patch5.47 KB

Darn, I completely lost track of this :/ I'm very sorry!

Rerolling.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs review » Reviewed & tested by the community

Looks good to me :) Pretty simple changes.

rszrama’s picture

rszrama CreditAttribution: rszrama commented

Yep, patch applies clean and still gets the job done. : )

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Nice catch!

Committed and pushed to 8.x. Thanks!

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented
Issue tags: -sprint

Status: Fixed » Closed (fixed)

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