Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
At some point of the SafeMarkup refactoring we broke the custom text fields in Views so that they are unable to output markup without getting escaping problems.
Proposed resolution
Field value should be XSS filtered and marked as safe afterwards.
Remaining tasks
- Write patch (DONE by laurii)
- Review patch (done by joel.pittet)
- Manual testing (done by ibullock)
- RC Evaluation (done by bigjim)
User interface changes
API changes
Data model changes
Why this should be an RC target
This is an important feature for basic site building. The use of custom markup is likely needed on a high % of new Drupal 8 sites.
Possible disruptions to current Drupal 8 sites that worked around this issue using escaping, though seems a relatively minor, easily fixed issue.
Before
After
After
Comment | File | Size | Author |
---|---|---|---|
#27 | output-after-patching.png | 158.47 KB | ibullock |
#27 | output-before-patching.png | 228.51 KB | ibullock |
#27 | views-config.png | 220.16 KB | ibullock |
#20 | interdiff.txt | 584 bytes | lauriii |
#19 | outputting_markup_from-2579427-19.patch | 4.03 KB | lauriii |
Comments
Comment #2
lauriiiComment #4
lauriiiLast one wont fly
Comment #6
mbayntonFixes the issue for me. Does this warrant a test?
Comment #7
LoMo CreditAttribution: LoMo as a volunteer commented@mbaynton It has the "Needs tests" issue tag and I would say that the need is warranted (to help prevent regressions)
Comment #8
lauriiiComment #9
joelpittetThis should likely use ViewsRenderPipelineMarkup as it's in the pipeline so we can find these later when/if we find a better solution.
Comment #11
lauriiiHow about this one?
Comment #12
joelpittetWell it is rendering so it would be nice if it was a value object... but there is precedent to it like this with a
['#markup' => 'render array']
A couple of examples that use #markup already.
\Drupal\views\Plugin\views\field\Dropbutton
\Drupal\views\Plugin\views\field\LinkBase
If the tests fail, blame Lauri:P
Comment #13
joelpittetComment #15
lauriiiComment #19
lauriiiComment #20
lauriiiComment #21
joelpittetThis was the approach I was thinking of from the start. It marks the ViewsRenderPipelineness for later review.
Comment #25
xjmThanks for tagging this for rc target triage! To have committers consider it for inclusion in RC, we should add a statement to the issue summary of why we need to make this change during RC, including what happens if we do not make the change and what any disruptions from it are. We can add a section
<h3>Why this should be an RC target</h3>
to the summary.Comment #26
bigjim CreditAttribution: bigjim commentedEvaluated the issue for RC eligibility, felt it should be :D.
Made changes @xjm noted above to issue summary
Comment #27
ibullockI tested the patch manually and confirmed the issue is resolved by the patch. Added screenshots to the issue summary.
Comment #28
ibullockAdded a retest for php5.5_mysql5.5
Comment #29
mradcliffeI don't think this is rc eligible, but it should definitely be a candidate for rc target because it is a bug fix and not a coding standard, doc update, testing improvement or library update as written in https://www.drupal.org/core/d8-allowed-changes#rc.
Comment #30
webchickAgreed. One-line bug fix (+tests), fixing a regression that was introduced just before RC. Moving to RC Target and asking for forgiveness. ;)
Committed and pushed to 8.0.x. Thanks!
Comment #33
FiNeX CreditAttribution: FiNeX as a volunteer commentedHi, I've just discovered that even inline CSS styling is stripped from the global custom text field inside views. Is it a desidered behaviour?
Comment #34
lauriiiWe run that field through Xss:filterAdmin() which removes everything related to styles and javascript so it is expected behavior.
Comment #35
FiNeX CreditAttribution: FiNeX as a volunteer commentedOk, thanks :-)