Skip to content
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

Fixing issue with how the check is done for determining if templatePa… #11620

Merged
merged 1 commit into from Jan 25, 2018
Merged

Fixing issue with how the check is done for determining if templatePa… #11620

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/View/View.php
View file
Open in desktop
Expand Up @@ -1249,7 +1249,7 @@ protected function _getViewFileName($name = null)
if (strlen($this->subDir)) {
$subDir = $this->subDir . DIRECTORY_SEPARATOR;
// Check if templatePath already terminates with subDir
if (strrpos($templatePath, $subDir) == strlen($templatePath) - strlen($subDir)) {
if ($templatePath != $subDir && substr($templatePath, -(strlen($subDir))) == $subDir) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use strict !== and === when you are comparing strings now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't $templatePath and $subDir be equal sometimes ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When would that happen? Extension directories are generally lower case, like csv while template paths usually end with the controller name Csvs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dereuromark - yes, maybe === should be used.

@saeideng - I agree with mark that is shouldn't happen because $templatePath is using the controller name and the first letter is uppercase, whereas the $subDir is typically all lowercase.

@markstory - Wouldn't the test you added during this commit (115a354) cover this case as well?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does to some extent. But you have a new scenario that requires different code. With only the existing test, this change could be reverted and there wouldn't be a failing test to let us know there was a regression.

$subDir = '';
}
}
Expand Down