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

Comments

nicholaspaun’s picture

nicholaspaun commented
Status: Active » Needs review
StatusFileSize
new 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 commented
Status: Needs work » Needs review
StatusFileSize
new 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 commented
Status: Needs work » Needs review
StatusFileSize
new 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 🇧🇪🇪🇺
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 🇧🇪🇪🇺
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 commented
Status: Needs work » Needs review
StatusFileSize
new 2003684-9.contextual_pre_render_placeholder_attributes.patch5.01 KB

Attempting a quick re-roll.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue summary: View changes
Issue tags: +Spark, +sprint
StatusFileSize
new 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
commented
Status: Needs review » Reviewed & tested by the community

Looks good to me :) Pretty simple changes.

rszrama’s picture

rszrama commented

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

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
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 🇧🇪🇪🇺
commented
Issue tags: -sprint

Status: Fixed » Closed (fixed)

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