[WebProfilerBundle] Fix dump header not being displayed #48048

HypeMC · 2022-10-29T16:10:59Z

Q	A
Branch?	5.4
Bug fix?	yes
New feature?	no
Deprecations?	no
Tickets	Fix #48039
License	MIT
Doc PR	-

The bug occurs because of the following log message:

The "Symfony\Bridge\Doctrine\Logger\DbalLogger" class implements "Doctrine\DBAL\Logging\SQLLogger" that is deprecated Use {@see \Doctrine\DBAL\Logging\Middleware} or implement {@see \Doctrine\DBAL\Driver\Middleware} instead.

Since this is the first message and it contains the { character the dump header gets rendered (line 93):

symfony/src/Symfony/Bundle/WebProfilerBundle/Twig/WebProfilerExtension.php

Lines 88 to 104 in f9eaefa

    
           public function dumpLog(Environment $env, string $message, Data $context = null) 
        
           { 
        
               $message = twig_escape_filter($env, $message); 
        
               $message = preg_replace('/&quot;(.*?)&quot;/', '&quot;<b>$1</b>&quot;', $message); 
        
               if (null === $context || !str_contains($message, '{')) { 
        
                   return '<span class="dump-inline">'.$message.'</span>'; 
        
               } 
        
               $replacements = []; 
        
               foreach ($context as $k => $v) { 
        
                   $k = '{'.twig_escape_filter($env, $k).'}'; 
        
                   $replacements['&quot;<b>'.$k.'</b>&quot;'] = $replacements['&quot;'.$k.'&quot;'] = $replacements[$k] = $this->dumpData($env, $v); 
        
               } 
        
               return '<span class="dump-inline">'.strtr($message, $replacements).'</span>'; 
        
           }

However, since there is no actual placeholder to replace, the header is never displayed, but is marked as displayed and never rendered again:

symfony/src/Symfony/Component/VarDumper/Dumper/HtmlDumper.php

Lines 149 to 152 in f9eaefa

    
           protected function getDumpHeader() 
        
           { 
        
               $this->headerIsDumped = $this->outputStream ?? $this->lineDumper;

symfony/src/Symfony/Component/VarDumper/Dumper/HtmlDumper.php

Lines 944 to 952 in f9eaefa

    
           protected function dumpLine(int $depth, bool $endOfValue = false) 
        
           { 
        
               if (-1 === $this->lastDepth) { 
        
                   $this->line = sprintf($this->dumpPrefix, $this->dumpId, $this->indentPad).$this->line; 
        
               } 
        
               if ($this->headerIsDumped !== ($this->outputStream ?? $this->lineDumper)) { 
        
                   $this->line = $this->getDumpHeader().$this->line; 
        
               }

With this PR, the WebProfilerExtension::dumpLog() method will check the message for exact placeholders.

PhilETaylor · 2022-10-30T11:50:06Z

I have tested this and it fixes the JS issue reported perfectly. Thanks.

However, probably unrelated, there is another bug I see - watch:

Screen.Recording.2022-10-30.at.11.49.09.mov

HypeMC · 2022-10-30T12:31:43Z

I have tested this and it fixes the JS issue reported perfectly. Thanks.

However, probably unrelated, there is another bug I see - watch:

Screen.Recording.2022-10-30.at.11.49.09.mov

@PhilETaylor Hi, thanks for the feedback. Unfortunately I'm unable to view your video on my computer (probably cause I'm on linux), is there any chance you could describe the issue or send some screenshots or gifs or something?

PhilETaylor · 2022-10-30T12:35:59Z

Sorry, here it is at Youtube so you should be able to view that right https://youtu.be/h5ztf03Zhjw ? The bug is at 7 seconds in when clicking the > next to the foreach loop, which, instead of collapsing the foreach loop next to the > goes on to close an unrelated dump in the trace.

HypeMC · 2022-10-31T12:42:12Z

Sorry, here it is at Youtube so you should be able to view that right https://youtu.be/h5ztf03Zhjw ? The bug is at 7 seconds in when clicking the > next to the foreach loop, which, instead of collapsing the foreach loop next to the > goes on to close an unrelated dump in the trace.

@PhilETaylor This looks like a javascript issue. I think it was introduced in #33486, but I'd have to look into it a little deeper. In any case, it's not related to this issue.

nicolas-grekas · 2022-12-13T10:48:31Z

Oops, can you please have a look at failures on deps=low?

HypeMC · 2022-12-13T11:51:02Z

@nicolas-grekas Sorry about that, must've missed that one, should be good now 68dc72d . I've also rebased the branch since it was originally intended for 4.4.

nicolas-grekas · 2022-12-13T13:10:13Z

Thank you @HypeMC.

carsonbot added Status: Needs Review Bug WebProfilerBundle labels Oct 29, 2022

carsonbot added this to the 5.4 milestone Oct 29, 2022

HypeMC force-pushed the profiler-dump-header branch from 6668bd3 to 2f2d6d3 Compare October 29, 2022 16:40

HypeMC changed the base branch from 5.4 to 4.4 October 29, 2022 16:41

HypeMC force-pushed the profiler-dump-header branch from 2f2d6d3 to 21f3389 Compare October 29, 2022 16:44

PhilETaylor mentioned this pull request Oct 30, 2022

[VarDumper] Can't find variable: Sfdump JS Error #48039

Closed

HypeMC changed the base branch from 4.4 to 5.4 November 28, 2022 19:43

nicolas-grekas approved these changes Dec 13, 2022

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Dec 13, 2022

HypeMC force-pushed the profiler-dump-header branch 2 times, most recently from 7094692 to 68dc72d Compare December 13, 2022 11:35

[WebProfilerBundle] Fix dump header not being displayed

81a4e9d

nicolas-grekas force-pushed the profiler-dump-header branch from 68dc72d to 81a4e9d Compare December 13, 2022 13:09

nicolas-grekas merged commit af562a7 into symfony:5.4 Dec 13, 2022

HypeMC deleted the profiler-dump-header branch December 13, 2022 13:16

fabpot mentioned this pull request Dec 16, 2022

Release v6.2.2 #48687

Merged

This was referenced Dec 28, 2022

Release v5.4.17 #48806

Merged

Release v6.0.17 #48807

Merged

Release v6.1.9 #48808

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WebProfilerBundle] Fix dump header not being displayed #48048

[WebProfilerBundle] Fix dump header not being displayed #48048

HypeMC commented Oct 29, 2022 •

edited

Loading

PhilETaylor commented Oct 30, 2022

HypeMC commented Oct 30, 2022

PhilETaylor commented Oct 30, 2022

HypeMC commented Oct 31, 2022

nicolas-grekas commented Dec 13, 2022

HypeMC commented Dec 13, 2022

nicolas-grekas commented Dec 13, 2022

	public function dumpLog(Environment $env, string $message, Data $context = null)
	{
	$message = twig_escape_filter($env, $message);
	$message = preg_replace('/"(.*?)"/', '"<b>$1</b>"', $message);

	if (null === $context \|\| !str_contains($message, '{')) {
	return '<span class="dump-inline">'.$message.'</span>';
	}

	$replacements = [];
	foreach ($context as $k => $v) {
	$k = '{'.twig_escape_filter($env, $k).'}';
	$replacements['"<b>'.$k.'</b>"'] = $replacements['"'.$k.'"'] = $replacements[$k] = $this->dumpData($env, $v);
	}

	return '<span class="dump-inline">'.strtr($message, $replacements).'</span>';
	}

	protected function getDumpHeader()
	{
	$this->headerIsDumped = $this->outputStream ?? $this->lineDumper;

	protected function dumpLine(int $depth, bool $endOfValue = false)
	{
	if (-1 === $this->lastDepth) {
	$this->line = sprintf($this->dumpPrefix, $this->dumpId, $this->indentPad).$this->line;
	}
	if ($this->headerIsDumped !== ($this->outputStream ?? $this->lineDumper)) {
	$this->line = $this->getDumpHeader().$this->line;
	}

[WebProfilerBundle] Fix dump header not being displayed #48048

[WebProfilerBundle] Fix dump header not being displayed #48048

Conversation

HypeMC commented Oct 29, 2022 • edited Loading

PhilETaylor commented Oct 30, 2022

HypeMC commented Oct 30, 2022

PhilETaylor commented Oct 30, 2022

HypeMC commented Oct 31, 2022

nicolas-grekas commented Dec 13, 2022

HypeMC commented Dec 13, 2022

nicolas-grekas commented Dec 13, 2022

HypeMC commented Oct 29, 2022 •

edited

Loading