8000 Web Profiler code not beeing injected properly · Issue #42248 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Web Profiler code not beeing injected properly #42248

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
537mfb opened this issue Jul 23, 2021 · 2 comments · Fixed by #42249
Closed

Web Profiler code not beeing injected properly #42248

537mfb opened this issue Jul 23, 2021 · 2 comments · Fixed by #42249
Labels
Bug WebProfilerBundle

Comments

@537mfb
Copy link
537mfb commented Jul 23, 2021 •
edited by Nyholm
Loading

Symfony version(s) affected: 5.4.x-DEV

Description
When openning a page in the dev environment, the web profiler tool isn't showing and there are errors in the browsers console saying Sfjs is not defined and there are sintax errors

Analysis of the generated html reveals that near the end of the first script block there's a comment using // instead of /* */ - and since the code is printed in one line, everything past that comment gets commented out until the closing of the script block

const selectedPriorities = Array.from(priorities).filter((input) => input.checked).map((input) => input.value);
const channels = document.querySelectorAll('#log-filter-channel input');
const selectedChannels = Array.from(channels).filter((input) => input.checked).map((input) => input.value);
const logs = document.querySelector('table.logs');
if (null === logs) {
    return;
}
// hide rows that don't match the current filters
let numVisibleRows = 0;
logs.querySelectorAll('tbody tr').forEach((row) => {
    if ('all' !== selectedType && selectedType !== row.getAttribute('data-type')) {
        row.style.display = 'none';
        return;
    }
    if (false === selectedPriorities.includes(row.getAttribute('data-priority'))) {
        row.style.display = 'none';
        return;
    }
    if ('' !== row.getAttribute('data-channel') && false === selectedChannels.includes(row.getAttribute('data-channel'))) {
        row.style.display = 'none';
        return;
    }
    row.style.display = 'table-row';
    numVisibleRows++;
});
document.querySelector('table.logs').style.display = 0 === numVisibleRows ? 'none' : 'table';

document.querySelector('.no-logs-message').style.display = 0 === numVisibleRows ? 'block' : 'none';

// update the selected totals of all filters
document.querySelector('#log-filter-priority .filter-active-num').innerText = (priorities.length === selectedPriorities.length) ? 'All' : selectedPriorities.length;
document.querySelector('#log-filter-channel .filter-active-num').innerText = (channels.length === selectedChannels.length) ? 'All' : selectedChannels.length;

// update the currently selected "log type" tab
document.querySelectorAll('#log-filter-type li').forEach((tab) => tab.classList.remove('active'));
document.querySelector(`#log-filter-type input[value="${selectedType}"]`).parentElement.classList.add('active');

            },        
};    
})();    
Sfjs.addEventListener(document, 'DOMContentLoaded', function() {
	Sfjs.createTabs();        
	Sfjs.createToggles();    
});}/*]]>*/</script>

How to reproduce
Create a new project using 5.4.x@dev
composer create-project symfony/website-skeleton:"5.4.x@dev" myproject

Configure a dummy DATABASE_URL in .env to overcome the error about not beeing able to connect to the database and create a new controller
bin/console make:controller HomeController

Access the controller via the browser - the profiler isn't shown and there are errors in the console

Possible Solution
Fix the comment to use /* */ instead of //

Additional context
As a side note - 5.4.x has a requirement of PHP >= 7.2.5 set in composer.json but if i try to access it with anything below PHP 8.0.0 I get an error saying it has dependencies that require PHP >= 8.0.0 - shouldn't 5.4.x require PHP >= 8.0.0 in that case?
@537mfb 537mfb added the Bug label Jul 23, 2021
@Nyholm Nyholm mentioned this issue Jul 24, 2021
Merged
@Nyholm
Copy link
Member
Nyholm commented Jul 24, 2021

You are the best!

Thank you for testing out unreleased code.
Your analysis is correct. I've fixed the comment in #42249

As a side note - 5.4.x has a requirement of PHP >= 7.2.5 set in composer.json but if i try to access it with anything below PHP 8.0.0 I get an error saying it has dependencies that require PHP >= 8.0.0 - shouldn't 5.4.x require PHP >= 8.0.0 in that case?

This seams like a separate issue. Im not sure why you experience this. Could you open a new issue with this comment? Maybe even find out where in the code this exception is thrown (or message is created).

@537mfb
Copy link
Author
537mfb commented Jul 24, 2021 •
edited
Loading

@nykopol Thanks for the quick fix on this

I'm still trying to figure out which dependencies require PHP8 - but sure i can open a new issue
just wasn't sure if this was by design and just that composer.json hadn't been updated yet

EDIT - added issue #42250

@fabpot fabpot closed this as completed Jul 24, 2021
fabpot added a commit that referenced this issue Jul 24, 2021
@fabpot
bug #42249 [WebProfilerBundle] Use multi line comment (Nyholm)
f5bc4a1 
This PR was merged into the 5.4 branch.

Discussion
----------

[WebProfilerBundle] Use multi line comment

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

Since the css/js is "fake minified" (we just remove the new line character) we cannot use `//`. See comment in the head of `base_js.html.twig`:

```
  {# Caution: the contents of this file are processed by Twig before loading
   them as JavaScript source code. Always use '/*' comments instead
   of '//' comments to avoid impossible-to-debug side-effects #}
```

I also added a small test to enforce this.

Commits
-------

f7d516e [WebProfilerBundle] Use multi line comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug WebProfilerBundle
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[WebProfilerBundle] Use multi line comment Nyholm/symfony
3 participants
@fabpot @Nyholm @537mfb
0