Show absolute path to WSC installation in path selection overlay on DevtoolsProjectListPage#6515
Show absolute path to WSC installation in path selection overlay on DevtoolsProjectListPage#6515Fighter456 wants to merge 3 commits intoWoltLab:6.2from
DevtoolsProjectListPage#6515Conversation
|
I’m not sure what the point of adding the placeholder is, because a placeholder is merely a hint and not available for text completion? |
| <dt>{lang}wcf.acp.devtools.project.quickSetup.path{/lang}</dt> | ||
| <dd> | ||
| <input type="text" name="projectQuickSetupPath" id="projectQuickSetupPath" class="long" /> | ||
| <input type="text" name="projectQuickSetupPath" id="projectQuickSetupPath" class="long" placeholder="{unsafe:$wcfDir}" /> |
There was a problem hiding this comment.
Why would you use unsafe: here?
There was a problem hiding this comment.
IIRC I've copied it from another template. Not necessary at this point?
There was a problem hiding this comment.
As a general guideline: Never use unsafe: unless you actually need it. The performance gains are negligible but you risk malformed HTML or XSS. This also makes it much easier to test for security issues because in HTML any variable that is escaped before outputting is safe by definition.
As stated in the PR description, the What about show the path in a |
|
Thanks. I’ve opted to make the changes myself since there I made some small changes at didn’t want to bother you with them, the result is effectively the same. |
2 participants
During the test of Suite 6.2 I've discovered, that the path selection overlay does not provide the absolute path to the Suite Core installation on the
DevtoolsProjectListPage.Because the path is shown on the
DevtoolsProjectAddFormit makes sense to unify this behavior.Note: The usage of
\WCF_DIRdirectly in the template is disallowed. Therefore the page-class needs to assign this information through the template engine.