Skip to content

Conversation

@lyrixx
Copy link
Member

@lyrixx lyrixx commented Aug 25, 2013

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

When the exception message contains some HTML, this display is
broken.

I don't know enought about the differents Output classes.
Maybe we have to check the current Output class before
escaping. But for the moment, we allways format the message
with error tags, so I guess the escape is not an issue.

As this is a bug fix, should I re-open this PR against 2.2 branch ?

@lyrixx
Copy link
Member Author

lyrixx commented Aug 25, 2013

For the reccord, here is the output before my patch: (see [0m<p>)



�[37;41m                    �[0m
�[37;41m  [Exception]       �[0m
�[37;41m  Second exception  �[0m
�[37;41m                    �[0m




�[37;41m                                       �[0m
�[37;41m  [Exception]                          �[0m
�[37;41m  First exception �[0m<p>this is html</p>  
�[37;41m                                       �[0m


�[32mfoo3:bar�[0m


@fabpot
Copy link
Member

fabpot commented Aug 26, 2013

IIRC, we do have error messages that have some ANSI sequences.

@lyrixx
Copy link
Member Author

lyrixx commented Aug 26, 2013

ag 'Exception(.*<.*>.*)' src/ returns some matches:

src/Symfony/Component/Routing/Loader/XmlFileLoader.php
src/Symfony/Component/Validator/PropertyMetadataContainerInterface.php
src/Symfony/Component/Console/Tests/Fixtures/Foo3Command.php
src/Symfony/Component/Console/Application.php
src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml
src/Symfony/Bundle/TwigBundle/Resources/views/Exception/exception.html.twig
src/Symfony/Bundle/WebProfilerBundle/Resources/config/profiler.xml
src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/exception.html.twig
src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/logger.html.twig
src/Symfony/Bundle/FrameworkBundle/Resources/config/collectors.xml
src/Symfony/Bundle/SecurityBundle/Resources/config/security_listeners.xml

The only one which can be "interesting" is src/Symfony/Component/Console/Application.php

if (OutputInterface::VERBOSITY_VERBOSE <= $output->getVerbosity()) {
    $output->writeln('<comment>Exception trace:</comment>');
    // exception related properties
    $trace = $e->getTrace();

But it is not ;)

@fabpot
Copy link
Member

fabpot commented Sep 11, 2013

So, what's the status of this PR?

@lyrixx
Copy link
Member Author

lyrixx commented Sep 11, 2013

IMHO, it's merge-able.

@fabpot
Copy link
Member

fabpot commented Sep 11, 2013

ok, can you do the same PR on 2.2 and add a test when using a valid tag to prove that it does not mess up those?

@lyrixx
Copy link
Member Author

lyrixx commented Sep 11, 2013

What is a valid tag ? something like <comment> foo bar </comment> ?

@fabpot
Copy link
Member

fabpot commented Sep 11, 2013

Everything that is registered as a style on OutputFormatter. I think the replacement code already does that. See the replaceStyle() method there.

jakzal and others added 6 commits September 13, 2013 13:11
This PR was merged into the 2.2 branch.

Discussion
----------

[Translation] Removed a @return annotation

This is the only `@return null` left...

Commits
-------

d6f4def [Translation] Removed an unneeded return annotation.
This PR was merged into the 2.2 branch.

Discussion
----------

[DomCrawler] Added missing docblocks and removed redundant type in a return annotation

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Commits
-------

d414213 [DomCrawler] Added missing docblocks and removed unneeded return annotation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants