Image Handler 4 (for v1.5.x) Support Thread

[mc12345678]

Yeah, but it is probably easier to change the module than change the auto-installer :-) You've got the line number form the errors

[04-Feb-2015 16:25:51 America/Chicago] PHP Warning: strstr(): Empty needle in /home/enignet/public_html/includes/classes/bmz_image_handler.class.php on line 129

You are probably going to do something like this

	function determine_image_sizetype() {
		global $ihConf;
		
		if (strstr($this->src, $ihConf['large']['suffix'])) {
			$this->sizetype = 'large';
		} elseif (strstr($this->src, $ihConf['medium']['suffix'])) {
			$this->sizetype = 'medium';
		} elseif ((intval($this->width) == intval($ihConf['small']['width'])) && (intval($this->height) == intval($ihConf['small']['height']))) {
			$this->sizetype = 'small';
		} else
		$this->sizetype = 'generic';
	}

changed to:

	function determine_image_sizetype() {
		global $ihConf;
		
		$this->sizetype = 'generic';

		if($ihConf['large']['suffix'] != '')
		{
			if (strstr($this->src, $ihConf['large']['suffix'])) {
				$this->sizetype = 'large';
			}			
		}

		if($ihConf['medium']['suffix'] != '')
		{
			if (strstr($this->src, $ihConf['medium']['suffix'])) {
				$this->sizetype = 'medium';
			}			
		}		
		
		if($ihConf['large']['small'] != '')
		{
			if (strstr($this->src, $ihConf['small']['suffix'])) {
				$this->sizetype = 'small';
			}			
		}
}

Off the top of my head and without looking at any of the other areas or consequences.

I would suggest either of the following two depending on the desired goal. First would be to prevent the error in this section of code, but wouldn't address similar potential problems later on, the second would set the variables in this section of code but also may still have the issue later, either way, an example to address the issue and maintain existing functionality. The example above appears to lead to all images being the size of the last defined size.

function determine_image_sizetype() {
global $ihConf;

if (zen_not_null($ihConf['large']['suffix']) && strstr($this->src, $ihConf['large']['suffix'])) {
$this->sizetype = 'large';
} elseif (zen_not_null($ihConf['medium']['suffix']) && strstr($this->src, $ihConf['medium']['suffix'])) {
$this->sizetype = 'medium';
} elseif ((intval($this->width) == intval($ihConf['small']['width'])) && (intval($this->height) == intval($ihConf['small']['height']))) {
$this->sizetype = 'small';
} else
$this->sizetype = 'generic';
}

function determine_image_sizetype() {
global $ihConf;
if (!zen_not_null($ihConf['large']['suffix']) {
$ihConf['large']['suffix'] = '_LRG';
} 

if (!zen_not_null($ihConf['medium']['suffix']) {
$ihConf['medium']['suffix'] = '_MED';
}

if (strstr($this->src, $ihConf['large']['suffix'])) {
$this->sizetype = 'large';
} elseif (strstr($this->src, $ihConf['medium']['suffix'])) {
$this->sizetype = 'medium';
} elseif ((intval($this->width) == intval($ihConf['small']['width'])) && (intval($this->height) == intval($ihConf['small']['height']))) {
$this->sizetype = 'small';
} else
$this->sizetype = 'generic';
}

[DivaVocals]

Yeah, but it is probably easier to change the module than change the auto-installer :-) You've got the line number form the errors

[04-Feb-2015 16:25:51 America/Chicago] PHP Warning: strstr(): Empty needle in /home/enignet/public_html/includes/classes/bmz_image_handler.class.php on line 129

You are probably going to do something like this

    function determine_image_sizetype() {
        global $ihConf;
        
        if (strstr($this->src, $ihConf['large']['suffix'])) {
            $this->sizetype = 'large';
        } elseif (strstr($this->src, $ihConf['medium']['suffix'])) {
            $this->sizetype = 'medium';
        } elseif ((intval($this->width) == intval($ihConf['small']['width'])) && (intval($this->height) == intval($ihConf['small']['height']))) {
            $this->sizetype = 'small';
        } else
        $this->sizetype = 'generic';
    }

changed to:

    function determine_image_sizetype() {
        global $ihConf;
        
        $this->sizetype = 'generic';

        if($ihConf['large']['suffix'] != '')
        {
            if (strstr($this->src, $ihConf['large']['suffix'])) {
                $this->sizetype = 'large';
            }            
        }

        if($ihConf['medium']['suffix'] != '')
        {
            if (strstr($this->src, $ihConf['medium']['suffix'])) {
                $this->sizetype = 'medium';
            }            
        }        
        
        if($ihConf['large']['small'] != '')
        {
            if (strstr($this->src, $ihConf['small']['suffix'])) {
                $this->sizetype = 'small';
            }            
        }
}

Off the top of my head and without looking at any of the other areas or consequences.

I would suggest either of the following two depending on the desired goal. First would be to prevent the error in this section of code, but wouldn't address similar potential problems later on, the second would set the variables in this section of code but also may still have the issue later, either way, an example to address the issue and maintain existing functionality. The example above appears to lead to all images being the size of the last defined size.

function determine_image_sizetype() {
global $ihConf;

if (zen_not_null($ihConf['large']['suffix']) && strstr($this->src, $ihConf['large']['suffix'])) {
$this->sizetype = 'large';
} elseif (zen_not_null($ihConf['medium']['suffix']) && strstr($this->src, $ihConf['medium']['suffix'])) {
$this->sizetype = 'medium';
} elseif ((intval($this->width) == intval($ihConf['small']['width'])) && (intval($this->height) == intval($ihConf['small']['height']))) {
$this->sizetype = 'small';
} else
$this->sizetype = 'generic';
}

function determine_image_sizetype() {
global $ihConf;
if (!zen_not_null($ihConf['large']['suffix']) {
$ihConf['large']['suffix'] = '_LRG';
} 

if (!zen_not_null($ihConf['medium']['suffix']) {
$ihConf['medium']['suffix'] = '_MED';
}

if (strstr($this->src, $ihConf['large']['suffix'])) {
$this->sizetype = 'large';
} elseif (strstr($this->src, $ihConf['medium']['suffix'])) {
$this->sizetype = 'medium';
} elseif ((intval($this->width) == intval($ihConf['small']['width'])) && (intval($this->height) == intval($ihConf['small']['height']))) {
$this->sizetype = 'small';
} else
$this->sizetype = 'generic';
}

Happy to update this once you guys hash out the details of WHAT update to make..

[niccol]

:-)

Whatever. There is a typo in my code due copying and pasting but basically they will all work. Lots of ways of skinning a cat.

	function determine_image_sizetype() {
		global $ihConf;
		
		$this->sizetype = 'generic';

		if($ihConf['large']['suffix'] != '')
		{
			if (strstr($this->src, $ihConf['large']['suffix'])) {
				$this->sizetype = 'large';
			}			
		}

		if($ihConf['medium']['suffix'] != '')
		{
			if (strstr($this->src, $ihConf['medium']['suffix'])) {
				$this->sizetype = 'medium';
			}			
		}		
		
		if($ihConf['small']['suffix'] != '')
		{
			if (strstr($this->src, $ihConf['small']['suffix'])) {
				$this->sizetype = 'small';
			}			
		}
	}

And, I am not sure that the first of your two options would prevent the error because if you put :

if (zen_not_null($ihConf['large']['suffix']) && strstr($this->src, $ihConf['large']['suffix']))

The 'strstr' will still always get evaluated. And it is that which is causing the issues. That's why they are nested in my version.

[DivaVocals]

:-)

Whatever. There is a typo in my code due copying and pasting but basically they will all work. Lots of ways of skinning a cat.

    function determine_image_sizetype() {
        global $ihConf;
        
        $this->sizetype = 'generic';

        if($ihConf['large']['suffix'] != '')
        {
            if (strstr($this->src, $ihConf['large']['suffix'])) {
                $this->sizetype = 'large';
            }            
        }

        if($ihConf['medium']['suffix'] != '')
        {
            if (strstr($this->src, $ihConf['medium']['suffix'])) {
                $this->sizetype = 'medium';
            }            
        }        
        
        if($ihConf['small']['suffix'] != '')
        {
            if (strstr($this->src, $ihConf['small']['suffix'])) {
                $this->sizetype = 'small';
            }            
        }
    }

And, I am not sure that the first of your two options would prevent the error because if you put :

if (zen_not_null($ihConf['large']['suffix']) && strstr($this->src, $ihConf['large']['suffix']))

The 'strstr' will still always get evaluated. And it is that which is causing the issues. That's why they are nested in my version.

This is ME looking confused.. (as usual) Soooooooooooooooooooooooooooooooooo mom & dad.... Which one do I choose.. (looks at mom... cause she's leaning towards her code..)

[mc12345678]

:-)

Whatever. There is a typo in my code due copying and pasting but basically they will all work. Lots of ways of skinning a cat.

	function determine_image_sizetype() {
		global $ihConf;
		
		$this->sizetype = 'generic';

		if($ihConf['large']['suffix'] != '')
		{
			if (strstr($this->src, $ihConf['large']['suffix'])) {
				$this->sizetype = 'large';
			}			
		}

		if($ihConf['medium']['suffix'] != '')
		{
			if (strstr($this->src, $ihConf['medium']['suffix'])) {
				$this->sizetype = 'medium';
			}			
		}		
		
		if($ihConf['small']['suffix'] != '')
		{
			if (strstr($this->src, $ihConf['small']['suffix'])) {
				$this->sizetype = 'small';
			}			
		}
	}

And, I am not sure that the first of your two options would prevent the error because if you put :

if (zen_not_null($ihConf['large']['suffix']) && strstr($this->src, $ihConf['large']['suffix']))

The 'strstr' will still always get evaluated. And it is that which is causing the issues. That's why they are nested in my version.

One of the things about php is that in an if statement that has ands, it processes from left to right. When a false is encountered it ditches out and doesn't process the remaining... I chose zen_not_null to stay in line with typical ZC code. If that is not provide a satisfactory result, then would use != '' instead. But it is my understanding that if a variable is provided to zen_not_null that if it is a string = '' it returns false, if it is NULL that is provided it returns false. Either of these two values are expected for nothing being in the configuration setting depending on the mysql setup of allowing thie constant to be null or ''.

Further, the above suggested change of all ifs being evaluated and not one stopping subsequent evaluation when an earlier is identified as being "the one" depends on none of the other variables being set when entering this function. Thus, if all large, medium and small are set, then the result of the series of ifs is that the image will always be identified as small....

I haven't dug back deeper in the code to identify if only one of those is set upon entry into this function, which if not mistaken was also true when either suggestion was made based on previous comment.

[DivaVocals]

Well alrighty then... I'll wait for you two titans to battle it out.. I'll implement any change that ya'll feel is the best.. 'cause this is SURELY over my head to address with anything more than some Bactine
and a BandAid

[mc12345678]

Further, looking into: includes/functions/extra_functions/functions_bmz_image_handler.php, the $ihConf variable is set without discrimination of the requested image size. Therefore all sub variables of $ihConf[size]['suffix'] are assigned, although small is not assigned a value as evidence in the review of the dimensions that is performed in the third elseif...

[niccol]

What the original code actually does is take one single filename.

It then checks to see if it contains "_LRG"
It then checks to see if it contains "_MED"
It then checks to see it it contains "_SML"

( or whatever the prefixes in use actually are )

That is what one might aim to reproduce. Any comment that all filenames will result in small being chosen is nonsense. Small can only be chosen if the filename contains "_SML" . And if the filename contains "_SML" then in zen style it is definitely a small image.

Let's not loose track of what the function actually does.

Further, the above suggested change of all ifs being evaluated and not one stopping subsequent evaluation when an earlier is identified as being "the one" depends on none of the other variables being set when entering this function. Thus, if all large, medium and small are set, then the result of the series of ifs is that the image will always be identified as small....

That would be true if it was working against an image. But it isn't. It is checking an individual path ( actually an image source ) to determine whether that path is 'large', 'medium', 'small' or 'generic'.

Diva, use whatever you choose. I typed some code off the top of my head. I have no emotional involvement in it. As often happens on this forum people seem intent on making some kind of stand that seems completely unnecessary. Not enough time in the world for my code is better than your code nonsense.

[mc12345678]

Agree, no time for "pumping chests" not worth it. I never did such, concerned about the accuracy of intention of the code and consideration of final result.

As said initially, it appeared that all results would end up as small. The fact is/was that there is a separate check once it is identified as not being empty, so yes the code provided (typos corrected which wasn't even a consideration in review) would at least check against the actual filename (src).

If however, and I don't recall the naming requirements identified in the instructions, the large and medium both are identified to have the same suffix (don't know why someone would do that, but same thing for having no suffix) then under the current code, the large image would be the default to display, whereas in the first suggested code, the medium would be the result with the sequence provided. So if going with the first suggested code, to maintain the current "result" the medium check should be ahead of the large check.

That stated, there is no defined small suffix. So that check would not work and result in the image being identified as the default 'generic' and not as a 'small' (disabling functions available to small images such as the zoom on hover).

I too don't care which code is "chosen" as long as it maintains functionality and prevents at least the identified error from occurring. With whatever change(s) are made, downstream effects need to be reviewed.

[DivaVocals]

What the original code actually does is take one single filename.

It then checks to see if it contains "_LRG"
It then checks to see if it contains "_MED"
It then checks to see it it contains "_SML"

( or whatever the prefixes in use actually are )

That is what one might aim to reproduce. Any comment that all filenames will result in small being chosen is nonsense. Small can only be chosen if the filename contains "_SML" . And if the filename contains "_SML" then in zen style it is definitely a small image.

Let's not loose track of what the function actually does.



That would be true if it was working against an image. But it isn't. It is checking an individual path ( actually an image source ) to determine whether that path is 'large', 'medium', 'small' or 'generic'.

Diva, use whatever you choose. I typed some code off the top of my head. I have no emotional involvement in it. As often happens on this forum people seem intent on making some kind of stand that seems completely unnecessary. Not enough time in the world for my code is better than your code nonsense.

I agree.. I was simply waiting for the dust to clear here a bit..

I'll implement and test the code you suggested (with the correction you later posted), and try the very scenario for which this is supposed to address along with all the other functionality to make sure there are no negative unforeseen impacts and let you know the result..

Niccol.. Thanks again.. I know this isn't your fav module, but honestly it was YOUR work that kept this thing standing when the v1.5.x Zen Cart release came along. I appreciate that you check in on us mere mortals here every now and then to make sure things are still okay.. As you KNOW I am truly only a POWERUSER of this module..The sum total of my CODE contributions to the "under the hood" inner workings of Image Handler is pretty small. My BIGGEST contribution has been to produce (what I think have been) better user docs for this thing.. The rest of this machine is not my doing.. I'm just the keeper of the Castle here..