Skip to content

Exception page in Symfony Profiler is broken after redesign #22442

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
sustmi opened this issue Apr 14, 2017 · 3 comments
Closed

Exception page in Symfony Profiler is broken after redesign #22442

sustmi opened this issue Apr 14, 2017 · 3 comments
Labels
Bug Status: Reviewed WebProfilerBundle

Comments

@sustmi
Copy link
Contributor

sustmi commented Apr 14, 2017 •
edited
Loading

Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no
Symfony version master

After the recent redesign of the exception page in PR #20951 the exception page in saved profile in Symfony Profiler (URL /_profiler/{token}?panel=exception) is broken.

See the screenshot:
broken-exception-page

It looks like rendering a saved profile uses only @Twig/Exception/exception.twig.html template without the @Twig/layout.html.twig that contains CSS for the page.
See

symfony/src/Symfony/Bundle/WebProfilerBundle/Controller/ExceptionController.php

Line 66 in bbe269f

$template,
called from

symfony/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/exception.html.twig

Line 33 in bbe269f

{{ render(path('_profiler_exception', { token: token })) }}
.

Therefore SVG "+" and "-" icons are displayed in default dimensions that are 1792x1792.
@sustmi
Copy link
Contributor Author

sustmi commented Apr 14, 2017 •
edited
Loading

The problem could be solved by changing exception to exception_full in

symfony/src/Symfony/Bundle/WebProfilerBundle/Controller/ExceptionController.php

Line 108 in bbe269f

return '@Twig/Exception/'.($this->debug ? 'exception' : 'error').'.html.twig';
but it just does not feel right because that would include whole HTML page (including <html>, <body> and <head> tags) into another page.

The other solution that comes into my mind is moving the in-line <style> from layout into exception.html.twig but this is also not the cleanest solution as <style> tags should be only inside <head> tag.
But most of the browsers support <style> tag inside <body> and it was very close from being tolerated by the standard: whatwg/html#1605 (comment) so I guess it would not be that bad after all.
What do you think?

@sustmi
Copy link
Contributor Author

sustmi commented Apr 14, 2017 •
edited
Loading

I have just found that @Twig/Exception/error.html that is rendered when debug is false (see

symfony/src/Symfony/Bundle/WebProfilerBundle/Controller/ExceptionController.php

Line 108 in bbe269f

return '@Twig/Exception/'.($this->debug ? 'exception' : 'error').'.html.twig';
) contains whole HTML page too. So I guess we do not have to worry about rendering @Twig/Exception/exception_full.html inside the Symfony Profiler page.

@curry684
Copy link
Contributor

Noticed this as well.

Status: reviewed

@e-moe e-moe mentioned this issue Apr 15, 2017
Closed
@javiereguiluz javiereguiluz mentioned this issue Apr 20, 2017
Merged
fabpot added a commit that referenced this issue Apr 22, 2017
@fabpot
bug #22491 Fixed the rendering of exceptions inside the profiler (jav…
dc92436 
…iereguiluz)

This PR was merged into the 3.3-dev branch.

Discussion
----------

Fixed the rendering of exceptions inside the profiler

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

In #22448 we tried to reuse the same CSS styles as the new exception pages. But that's not enough because it doesn't look good:

![before](https://cloud.githubusercontent.com/assets/73419/25250100/a287b4bc-2614-11e7-88e7-e0ef89eac1ff.png)

---

This PR goes a bit further and tweaks the exception styles slightly to better integrate them. Same page as before:

![after_1](https://cloud.githubusercontent.com/assets/73419/25250131/b8d16cf4-2614-11e7-93b4-187248849103.png)

It should also work reasonably well when the exception message is very long:

![after_2](https://cloud.githubusercontent.com/assets/73419/25250144/c25eab38-2614-11e7-99e2-843548d12810.png)

Commits
-------

73d81de Fixed the rendering of exceptions inside the profiler
@fabpot fabpot closed this as completed Apr 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Status: Reviewed WebProfilerBundle
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@fabpot @sustmi @curry684 @aitboudad @carsonbot