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

  1. Go to the Extend page and ensure that Views UI module is enabled.
  2. Go to the Views admin page (Admin > Structure > Views).
  3. Click on the Settings tab.
  4. In "Live Preview Settings", mark "Show the SQL query" and " Show performance statistics" and save the form.
  5. Go to the "List" tab.
  6. Edit the "Content" view.
  7. In the "Preview" panel, you will find that there are some <strong/> elements that should not be there.

Before:

After:

Contributor tasks needed
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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

penyaskito’s picture

Issue summary: View changes
penyaskito’s picture

penyaskito’s picture

Issue summary: View changes
gvso’s picture

Status: Active » Needs review
FileSize
1.78 KB
12.71 KB

This patch seems to fix it.

Here is an image after applying the patch.

penyaskito’s picture

Status: Needs review » Needs work
+++ b/core/modules/views_ui/src/ViewUI.php
@@ -710,10 +710,20 @@ public function renderPreview($display_id, $args = array()) {
+              intval($this->executable->build_time * 100000) / 100 . ' ms',
...
+              intval($this->executable->execute_time * 100000) / 100 . ' ms',
...
+              intval($this->executable->render_time * 100000) / 100 . ' ms',

This should use t() as before the patch ( t('@time ms', ...) ).

penyaskito’s picture

Issue tags: +Needs tests

Not sure if we need tests for double-escaping issues, but they won't harm anyway so tagging.

dawehner’s picture

Seems fine for me ... I don't think having a test for that makes sense, its already an int

gvso’s picture

Status: Needs work » Needs review
FileSize
1.85 KB

Using t()

wizonesolutions’s picture

Can'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.

wizonesolutions’s picture

Scratch #9. Steps are fine, and it was me having the devel module enabled in my build that did it.

Before and after attached.

+1

penyaskito’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs issue summary update, -Novice, -Needs tests +SprintWeekend2015

The patch looks good to me (thanks @gvso!).
Updated issue summary including the screenshoots (thanks @wizonesolutions!).
Removing "Needs tests" tag per #7.

DevElCuy’s picture

Issue tags: -SprintWeekend2015Queue
holingpoon’s picture

Issue tags: +Novice

Suggested missing steps on reproducing the bug:

Make sure to install and enable Devel module.

  1. In administrator mode, go to Configuration
  2. In the 'Devel' box, go to 'Devel settings'
  3. Under 'Query log', check the 'Display query log' checkbox. Save configuration.

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.

gvso’s picture

Thanks @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.

gvso’s picture

FileSize
78.33 KB

Here is an image reproducing the issue in simplytest.me without Devel module

holingpoon’s picture

@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.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

I 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!

  • webchick committed 0877653 on 8.0.x
    Issue #2406903 by gvso, wizonesolutions, penyaskito, holingpoon: HTML...

Status: Fixed » Closed (fixed)

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