[#43239] - [5.2] 5.2-Styles and Script element in rendering Debug Assets
- Pending
- Medium
- Build: 5.2-dev
- # 43239
- Diff
- sinahaghparast:5.2-StylesRenderer.php-JavascriptRenderer.php
User tests: Successful: Unsuccessful:
Pull Request for Issue # .
Summary of Changes
Code be better...
Testing Instructions
Actual result BEFORE applying this Pull Request
Validator message:
Info: The type attribute is unnecessary for JavaScript resources.
Info: Trailing slash on void elements has no effect and interacts badly with unquoted attribute values.
Expected result AFTER applying this Pull Request
Validator message:
Document checking completed. No errors or warnings to show.
Link to documentations
Please select:
-
Documentation link for docs.joomla.org:
-
No documentation changes for docs.joomla.org needed
-
Pull Request link for manual.joomla.org:
-
No documentation changes for manual.joomla.org needed
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Front End com_content Layout Libraries Plugins |
| Labels |
Added:
PR-5.2-dev
|
||
| Category | Front End com_content Layout Libraries Plugins | ⇒ | Layout Libraries Front End Plugins |
| Category | Front End Layout Libraries Plugins | ⇒ | Libraries Front End Plugins |
I removed itemprop...
I removed itemprop...
Now you need to make sure the pull request title and description are still correct and if not then you will need to edit those. The title is particularly important as that is what people will seein the changelog
| Title |
|
||||||
the javascript renderer for the debug plugin is not in the head. It is just before the closing body tag. You probably should also fix the css for that plugin as well.
The code for the debug css is here:
For the JS is here:
Also just reviewing the code the nonce attribute needs to be applied for each stylesheet/script, right now debug is probably broken with CSP enabled
Also, I don't agree with adding the nonce attribute to the inline style. Because according to this document, the code inside the elements are also blocked.
You misunderstood what the inline style it refers to. It's not the <style> tag but rather the attribute style="display: none".
FWIW Joomla is already adding the nonce for the head inline css/js tags.
If I have misunderstood, then you guide me so that I can be informed. Just stating that I got it wrong won't help me. It may be repeated until the offender realizes his mistake. Please introduce new document or interpret that document.
CSP has different levels. The strict level is the one that does not allow inline css (attribute style). I'm not sure how I can explain this better as the CSP chapter is pretty big and needs some studying to grasp the fundamentals. MDN and web.dev are good starting points, then more googling are my only suggestions here if the comments are not sufficient
I read a page from the MDN website. And I understood such a concept. Anyway, I trust your words. Are the current changes acceptable?
| Title |
|
||||||
| Labels |
Added:
Feature
|
||
trailing slash and needless attribute in debug bar removed :-)
I have tested this item ✅ successfully on b609e14
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43239.
This pull request has been automatically rebased to 5.3-dev.
| Title |
|
||||||
| Labels |
Added:
Information Required
PR-5.3-dev
Removed: PR-5.2-dev |
||
| Category | Front End Libraries Plugins | ⇒ | Front End Plugins |
| Title |
|
||||||
@brianteeman
title and code edited.
@brianteeman title and code edited.
the title still says this is for the head
@brianteeman title and code edited.
the title still says this is for the head
@brianteeman
renderHead is function's name...
What do you recommend?
@sinahaghparast the debugbar injects the assets at the end of the body (although the class says head) so maybe instead of head should say debugbar or just debug?
@brianteeman title and code edited.
the title still says this is for the head
@brianteeman renderHead is function's name... What do you recommend?
I suggest to name the function "renderDebugAssets".
Is okay?
| Title |
|
||||||
I suggest to name the function "renderDebugAssets". Is okay?
@sinahaghparast I would not change the method name for b/c (backwards compatibility) reasons. But the title of your pull request is better now.
Before jumping in and "fixing" the code you really need to spend a little more time both using Joomla dn looking through the code history.
For example you will find that the "itemprop" you are adding here has only just been removed because we switched to use json rich snippets see #41151 so adding it here is completely wrong.
Secondly a pull request should address a single issue. Here you are addressing two unrelated issues