Problem/Motivation
When showing views debug info, the revision info is double escaped.
See #2297711: Fix HTML escaping due to Twig autoescape and https://www.drupal.org/node/2311123 for backstory.
Proposed resolution
Use #inline_template in \Drupal\views_ui\ViewUI::renderPreview
for those strings instead of concatenating its output.
See how "Query" string is done some lines above in the same method.
Steps to reproduce
- Go to the Extend page and ensure that Views UI module is enabled.
- Go to the Views admin page (Admin > Structure > Views).
- Click on the Settings tab.
- In "Live Preview Settings", mark "Show the SQL query" and " Show performance statistics" and save the form.
- Go to the "List" tab.
- Edit the "Content" view.
- In the "Preview" panel, you will find that there are some
<strong/>
elements that should not be there.
Before:
After:
Task | Novice task? | Contributor instructions | Complete? |
---|---|---|---|
Update the issue summary | Novice | Instructions | Y |
Manually test the patch | Novice | Instructions | Y |
Add steps to reproduce the issue | Novice | Instructions | Y |
Embed before and after screenshots in the issue summary | Novice | Instructions | Y |
Review patch to ensure that it fixes the issue, stays within scope, is properly documented, and follows coding standards | Instructions | Y |
Comment | File | Size | Author |
---|---|---|---|
#15 | double-scaping-no-devel.png | 78.33 KB | gvso |
#10 | AFTER-Frontpage__Content____Drupal_Test_Site.jpg | 149.16 KB | wizonesolutions |
#10 | BEFORE-Frontpage__Content____Drupal_Test_Site.jpg | 152.16 KB | wizonesolutions |
#4 | double-scaping-view-debug-2406903-3.patch | 1.78 KB | gvso |
#4 | no-double-scaping.png | 12.71 KB | gvso |
Comments
Comment #1
penyaskitoComment #2
penyaskitoComment #3
penyaskitoComment #4
gvsoThis patch seems to fix it.
Here is an image after applying the patch.
Comment #5
penyaskitoThis should use t() as before the patch (
t('@time ms', ...)
).Comment #6
penyaskitoNot sure if we need tests for double-escaping issues, but they won't harm anyway so tagging.
Comment #7
dawehnerSeems fine for me ... I don't think having a test for that makes sense, its already an int
Comment #8
gvsoUsing t()
Comment #9
wizonesolutionsCan't reproduce; seems like one also needs to have some content in the system? Attempting that, and will update steps to reproduce if that's what was missing.
Comment #10
wizonesolutionsScratch #9. Steps are fine, and it was me having the devel module enabled in my build that did it.
Before and after attached.
+1
Comment #11
penyaskitoThe patch looks good to me (thanks @gvso!).
Updated issue summary including the screenshoots (thanks @wizonesolutions!).
Removing "Needs tests" tag per #7.
Comment #12
DevElCuy CreditAttribution: DevElCuy commentedComment #13
holingpoon CreditAttribution: holingpoon commentedSuggested missing steps on reproducing the bug:
Make sure to install and enable Devel module.
After that, proceed with origin 'Steps to Reproduce'
The Devel module is not installed on Drupal 8 by default, so no assumptions should be made that a novice have already installed the module before proceeding with steps to reproduce the bug. Without 'display query log' enabled, the queries did not show up on the Preview of Content View settings.
Comment #14
gvsoThanks @holingpoon although I don't think devel is necessary in order to reproduce this issue, the view module allows to see those queries and actually I reproduced this issue in a test installation and didn't have devel.
Comment #15
gvsoHere is an image reproducing the issue in simplytest.me without Devel module
Comment #16
holingpoon CreditAttribution: holingpoon commented@gvso, thank you for your feedback!
On my local install, I can't seem to reproduce the bug without devel module. I have followed 'Steps to reproduce', but the Preview never appeared with the query even when I populated content on the local instance. I got a hint from #10 that I should install devel module. No installation or configuration instructions were included, hence the comment in #13. I hope folks dropping by this forum will find the info in #13 helpful.
Comment #17
webchickI was going to whine a little bit about the array formatting matching coding standards, but I see that elsewhere in ViewsUi.php it does the same thing. So filed another novice issue at #2409391: Fix the whitespace of inline_template declarations in Views UI to clean that up, and accidentally uncovered #2409395: Fix typo 'lama' => 'llama' in the process — novice-ception!
Looks like some folks are having a bit of a hard time reproducing this, but OTOH it doesn't look like it'll hurt anything, and it brings these messages inline with others in the same file.
Committed and pushed to 8.0.x. Thanks!