-
-
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
protected function _isApplicableAttribute #2829
Comments
I am starting to consider that it's not the extension that's to blame for the error but the OpenMage code related to the PHP version used. 7.0: is running fine. no errors appear in the system.log and in the browser window Warning: count(): Parameter must be an array or an object that implements Countable in /var/www/html/app/code/core/Mage/Catalog/Model/Resource/Abstract.php on line 71 8.0: is not running. the fatal error appears in the browser, nothing in the system.log I wanted to find out the value of the $applyTo variable. I insert var_dump()and this is what it displays in browser Frontend: array(0) { } If I convert the variable to array this is what appears in browser Frontend: array(1) { [0]=> array(0) { } } It is obvious that using the count() function in these conditions will generate an error. Searching the Internet for the fatal error I found a lot of pages where the solution is to convert the type to array. Once done in the OpenMage code, the error disappear and extension works without issues. |
Here is what appears in system.log with PHP 8.2 when loading the product page in Frontend 2022-12-21T10:41:32+00:00 ERR (3): Warning: Invalid argument supplied for foreach() in /var/www/html/app/design/frontend/rwd/default/template/catalog/product/view/media.phtml on line 42
2022-12-21T10:41:32+00:00 ERR (3): Warning: count(): Parameter must be an array or an object that implements Countable in /var/www/html/app/design/frontend/rwd/default/template/catalog/product/view/media.phtml on line 53 Changing in both lines from $this->getGalleryImages() to (array)$this->getGalleryImages() the errors disappear from system.log. In my case it's value was NULL having no product images loaded. It seems this error is a known one. I certainly wouldn't have found out about it if I hadn't tested this extension. There are a lot of people asking for help in the last year in different PHP forums. Here is one resource |
Let's execute the code bellow on https://onlinephp.io/ <?php
$variableTest = NULL;
var_dump(count($variableTest)); PHP 7.0.33, 7.1.33 int(0) PHP 7.2.34, 7.3.33, 7.4.33 Warning: count(): Parameter must be an array or an object that implements Countable in /home/user/scripts/code.php on line 3
int(0) PHP > 8.0 Fatal error: Uncaught TypeError: count(): Argument #1 ($value) must be of type Countable|array, null given in /home/user/scripts/code.php:3
Stack trace:
#0 {main}
thrown in /home/user/scripts/code.php on line 3 Because the minimum requirements of the OM are already for PHP set to version 7.3 I would propose to modify the OpenMage code in the case of the count() function to deal specifically with the situation when a variable has the NULL value. Using array($variable) is not quite suitable as a solution but one just to see an immediate effect, although if it is NULL it becomes an empty array. Also, the replacement with !empty($variable) or isset($variable) doesn't seem right to me. In PHP 7.3 the is_countable() function was introduced, but we already have from end of May a polyfill introduced here #2163. I would propose that the change be as follows count($variable) => count((is_countable($variable) ? $variable : [] )) We can also check the variable directly in an if condition with !is_null($variable). I searched OM for this topic and I found a few discussions and PRs but not fixing this one. |
maybe some module is overriding the getGalleryImages() method or something like that? |
At this moment the extension is disabled by editing its module file. I was able to reproduce this issue pretty simple, by renaming the media directory then creating an empty one. OM thinks the products have images but there are not physically accessible. This issue is not related to the one reported here, I discovered it by chance. Using count function with NULL values is really an important issue. Even we fix this one there a lot in the code. Now I realize that many do not upgrade the operating system and for this reason we do not have reports, but when they will do it we will definitely identify others count related issues. |
Cant reproduce it with 1.9.4.x / PHP8.1 When media directory is empty and i go to product page, placeholder images are added. |
No problem. I will spend some time on this issue. I have in plan more tests. |
Can you add a xdebug breakpoint there an post stacktrace? (or share code?) |
When I last checked the extension it was in PHP 7.0.33 and it was functional, but since then almost 1 year has passed and the developer from Ukraine no longer responds. I hope with all my heart that he is in good health because surely the war started by an idiot either created problems for him or led him to war. |
❤️ @Flyingmana OpenMage/Web_Notifications#4 ... not? |
I used XDebug together with the extension and the value of the $applyTo variable is null. Using the null value in the count() function creates trouble starting with PHP 7.2 (warnings in system.log) and PHP 8.0 (fatal error in browser). For this reason I would insert a line before return to prevent this particular case, if the value is null then I set it to an empty array. In the OM file /app/code/core/Mage/Catalog/Model/Resource/Abstract.php in the _isApplicableAttribute method (line 68) here is the change /**
* Check whether the attribute is Applicable to the object
*
* @param Varien_Object $object
* @param Mage_Catalog_Model_Resource_Eav_Attribute $attribute
* @return bool
*/
protected function _isApplicableAttribute($object, $attribute)
{
$applyTo = $attribute->getApplyTo();
if(is_null($applyTo)) $applyTo = [];
return count($applyTo) == 0 || in_array($object->getTypeId(), $applyTo);
} The use of the is_countable() function is not appropriate in this case. With my change the extension works, there are no more warnings or errors. |
After some investigation ... IMHO ... this problem comes from this extension. (edit: and OM)
Adding this to
edit: added PR |
I am running in a test environment with DDEV the latest OM 20 + PHP 7.3 (requested by Composer). I installed an old extension which worked great with PHP 7.0 to update it to 8.2 but OM is not loading some sections that this extension adds to the Backend.
In the browser window I am getting a fatal error like this:
Based on the error I checked the /app/code/core/Mage/Catalog/Model/Resource/Abstract.php file on line 70. Here is the PHP code:
If I change it as follows
the fatal error disappears and there are no more errors in /var/log/system.log file from now on like I got with PHP 7.3.
As you may see I am editing an OpenMage file to make this extension working properly. Do you consider this an issue into the OM code related to new PHP versions we need to take in consideration? Maybe PHPStan could solve this mystery but I am not an expert, honestly speaking I never used it.
The text was updated successfully, but these errors were encountered: