JCgH4164838Gh792C124B5 commented on a change in pull request #434:
URL: https://github.com/apache/struts/pull/434#discussion_r485180192
##########
File path:
core/src/main/resources/org/apache/struts2/interceptor/debugging/console.ftl
##########
@@ -21,7 +21,9 @@
<!DOCTYPE html>
<html>
<head>
- <script type="text/javascript">
+ <script type="text/javascript" <#rt/>
+ <#include "/${parameters.templateDir}/simple/nonce.ftl" />
Review comment:
Should the changes being made in this PR be collapsed into single lines
(where possible) to match the visual structure of the original tag(s) ?
For example in this case, utilize a single line such as:
`<script type="text/javascript" <#include
"/${parameters.templateDir}/simple/nonce.ftl" /> >`
to replace the three lines would eliminate the need for extra `<#rt/>` tag
processing in the template and produce a visual result closer to the original
tag.
##########
File path: core/src/main/resources/template/css_xhtml/head.ftl
##########
@@ -18,5 +18,7 @@
* under the License.
*/
-->
-<link rel="stylesheet" href="<@s.url value='/struts/css_xhtml/styles.css'
includeParams='none' encode='false' />" type="text/css" />
+<link
+ <#include "/${parameters.templateDir}/simple/nonce.ftl" />
Review comment:
And here:
`<link <#include "/${parameters.templateDir}/simple/nonce.ftl" />
rel="stylesheet" href="<@s.url value='/struts/css_xhtml/styles.css'
includeParams='none' encode='false' />" type="text/css" />`
This sort of structure repeats in several of the PR's code changes, so those
could be modified in a similar way if it makes sense.
##########
File path: core/src/main/resources/template/simple/form-close-tooltips.ftl
##########
@@ -24,7 +24,11 @@
--><#t/>
<#if (parameters.hasTooltip!false)><#t/>
<#lt/><!-- javascript that is needed for tooltips -->
- <#lt/><script type="text/javascript" src='<@s.url
value="/struts/domTT.js" includeParams="none" encode="false" />'></script>
- <#lt/><link rel="stylesheet" type="text/css" href="<@s.url
value="/struts/domTT.css" includeParams="none" encode="false" />"/>
+ <#lt/><script type="text/javascript" src='<@s.url
value="/struts/domTT.js" includeParams="none" encode="false" />'<#rt/>
Review comment:
If the `<#rt/>` tag structure is kept, should this one include a space
to be consistent with the others ?
i.e. `/>' <#rt/>`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
