-
-
Notifications
You must be signed in to change notification settings - Fork 436
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.
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
Some changes for compatibility with PHP 8.1 #1812
Changes from all commits
2040871
fcd55eb
2ff0203
71d384e
ba300ae
d196a33
11fa00b
c070dc9
70e19ae
b1597cf
b1950b2
cedc039
a669060
f53c0e3
8fd3228
7c92977
75841c2
aef2d26
7a4aa96
d81b47f
060654b
88cad3f
82ffa4c
77c582d
274f65a
c2e12d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -140,8 +140,8 @@ public function getStoreLastLoginDateTimezone() | |
public function getCurrentStatus() | ||
{ | ||
$log = $this->getCustomerLog(); | ||
if ($log->getLogoutAt() || | ||
strtotime(now())-strtotime($log->getLastVisitAt())>Mage_Log_Model_Visitor::getOnlineMinutesInterval()*60) { | ||
if ($log->getLogoutAt() || (!empty($log->getLastVisitAt()) && | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This fix removes the deprecation error but introduces a bug. Up to the fix the behavior has been that I fixed it by combining your check with correctly executing the subtraction: if ($log->getLogoutAt()
|| strtotime(now()) - (!empty($log->getLastVisitAt()) ? strtotime($log->getLastVisitAt()) : 0) > Mage_Log_Model_Visitor::getOnlineMinutesInterval() * 60) {
return Mage::helper('customer')->__('Offline');
} |
||
(strtotime(now()) - strtotime($log->getLastVisitAt()) > Mage_Log_Model_Visitor::getOnlineMinutesInterval() * 60))) { | ||
return Mage::helper('customer')->__('Offline'); | ||
} | ||
return Mage::helper('customer')->__('Online'); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -101,7 +101,7 @@ public function loadToXml(Mage_Core_Model_Config $xmlConfig, $condition = null) | |
if ($r['scope'] !== 'default') { | ||
continue; | ||
} | ||
$value = str_replace($substFrom, $substTo, $r['value']); | ||
$value = empty($r['value']) ? $r['value'] : str_replace($substFrom, $substTo, $r['value']); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This added empty-check must also be applied to line 121 and 150 for website and store scope, respectively. |
||
$xmlConfig->setNode('default/' . $r['path'], $value); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -214,7 +214,7 @@ public function removeClass($class) | |
*/ | ||
protected function _escape($string) | ||
{ | ||
return htmlspecialchars($string, ENT_COMPAT); | ||
return is_string($string) ? htmlspecialchars($string, ENT_COMPAT) : $string; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In general I'm happy with your PR mainly relying on added typechecks to avoid problems with PHP 8.1. But in this case I would like to argue that an explicit cast to string There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest
|
||
} | ||
|
||
/** | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.