probable bugs in filter.php

  • Posts: 96
  • Thank you received: 6
  • Hikashop Business
4 years 7 months ago #310721

-- HikaShop version -- : 4.2.1
-- Joomla version -- : 3.9.11

This is Randy Carey, Karen's developer. I was resolving her recent issue as she reported here: www.hikashop.com/forum/install-update/89...ing-as-expected.html

In trying to get the filter to work, I stumbled upon two issues that are tied to the "Price" section within classes/fitler.php (section starts on line 1789).

First, I see that in order for the checkboxes to know what items are selected, the config value for "redirect_post" must be on. Without that setting on, the page would filter by what I checked, but the fitler would refresh with the checkbox unchecked. There may be a reason why the boxes should not be checked when redirect_post is off, but I assume the "checked" behavior should always occur. So it seems to me that the condition if($config->get('redirect_post')) should be removed so $selected is always set and the checkboxes always get checked correctly -regardless of the setting on RedirectPostMode. At least from my perspective. We resolved this by turning on the RedirectPostMode, but did so only to get past this problem.

Second, we noticed a warning in the console of non-unique ids. Where the list of checkboxes is built for Pricing options, the code's logic applies the same id value to the last item as it does to the second-to-last-item. I was able to resolve this by appending some text to the id for the last item. Obviously, the long-term solution should not have me modifying filter.php which is not overridable. This is not a critical issue, and this warning did not seem to cause any problems on the frontend, but I am assuming duplicate ids was not intended.

That's it. Since I had to dig into this code to discover what was causing a problem with our filter, I thought I'd report these two things.

Last edit: 4 years 7 months ago by karendunne.

Please Log in or Create an account to join the conversation.

  • Posts: 81481
  • Thank you received: 13062
  • MODERATOR
4 years 7 months ago #310745

Hi,

Thank you for your report.

However, I don't know which build of the 4.2.1 you have, so I'm not sure which lines you're refering to in filter.php
Could you provide the block of code where you did the change ?

Please Log in or Create an account to join the conversation.

  • Posts: 96
  • Thank you received: 6
  • Hikashop Business
4 years 7 months ago #310782

in file /components/com_hikashop/classes/flter.php ... where the condition is filter_data==price. Relevant code snippets below...

if(($filter->filter_data=='price' || $filter->filter_data=='information' || $filter->filter_data=='custom_field') && !empty($filter->filter_value)){
    if(parent::checkCurrency($filter)==false){ return false;}
          $size=count($filter->filter_value);
          if($size){
               $config = hikashop_config();
                [color=red]if($config->get('redirect_post')){[/color]
                        $selected = array_chunk($selected, 2);
                         foreach($selected as $k => $v){
                         $selected[$k] = implode('::', $v);
                 }
           }

In the code above, I assume the condition of $config->get('redirect_post') should not be included ... so that $selected can get loaded for use later. Allowing a this to not be set in some scenarios seems to be an error.

Then when the input boxes are created for price-range options...
if($key==0){
	if(!empty($selected) && is_array($selected) && in_array('::'.$value, $selected)){
		$checked='checked="checked"';
		if($type=='radio'){ $deleteButton='  '.parent::getDeleteButton($filter, $divName, '', $html, '', true, 'filter_'.$filter->filter_id.'_'.$value.'' ); }
	}
	$html.='<span class="hikashop_filter_checkbox"><input '.$onClick.' '.$checked.' type="'.$type.'" name="filter_'.$filter->filter_namekey.''.$tab.'" value="::'.$value.'" id="filter_'.$filter->filter_id.'_'.$value.'"/><label class="filter_'.$filter->filter_namekey.''.$tab.'" for="filter_'.$filter->filter_id.'_'.$value.'">'.JText::sprintf('X_AND_INFERIOR',$formatVal).'</label>'.$deleteButton.'</span>'.$br;
	parent::getDeleteButton($filter, $divName, '', $html, true);
}else{
	if(!empty($selected) && is_array($selected) && in_array($filter->filter_value[$key-1].'::'.$value, $selected)){
		$checked='checked="checked"';
		if($type=='radio'){ $deleteButton='  '.parent::getDeleteButton($filter, $divName, '', $html, '', true, 'filter_'.$filter->filter_id.'_'.$value.'' ); }
	}
	$html.='<span class="hikashop_filter_checkbox"><input '.$onClick.' '.$checked.' type="'.$type.'" name="filter_'.$filter->filter_namekey.''.$tab.'" value="'.$filter->filter_value[$key-1].'::'.$value.'" id="filter_'.$filter->filter_id.'_'.$value.'"/><label class="filter_'.$filter->filter_namekey.''.$tab.'" for="filter_'.$filter->filter_id.'_'.$value.'">'.JText::sprintf('FROM_X_TO_Y', $oldVal, $formatVal ).'</label>'.$deleteButton.'</span>'.$br;
}
if($key==$size-1){
	$checked=''; $deleteButton='';
	if(!empty($selected) && is_array($selected) && in_array($value.'::', $selected)){
		$checked='checked="checked"';
		if($type=='radio'){ $deleteButton='  '.parent::getDeleteButton($filter, $divName, '', $html, '', true, 'filter_'.$filter->filter_id.'_'.$value.''); }
	}
	$html.='<span class="hikashop_filter_checkbox"><input '.$onClick.' '.$checked.' type="'.$type.'" name="filter_'.$filter->filter_namekey.''.$tab.'" value="'.$value.'::" id="filter_'.$filter->filter_id.'_'.$value.'"/><label class="filter_'.$filter->filter_namekey.''.$tab.'" for="filter_'.$filter->filter_id.'_'.$value.'">'.JText::sprintf('X_AND_SUPERIOR', $formatVal ).'</label>'
}

In this code, the id for the second to last and last input is exactly the same. I was able to avoid this by building the id value this way:
from
id="filter_'.$filter->filter_id.'_'.$value.'"
to
id="filter_'.$filter->filter_id.'_'.$value.'_plus"
I applied this to the id= and to the for= attribute.
This makes the last item have a unique value instead of shared with that of the preceding input.

Please Log in or Create an account to join the conversation.

  • Posts: 81481
  • Thank you received: 13062
  • MODERATOR
4 years 7 months ago #310794

Hi,

Thanks for the additional information.

1. I've changed the code to :

if(is_array($selected) && count($selected) > 2){
That way, it should work when needed, regarding of that redirect_post setting.

2. Thank you. Indeed, I've made the change on our end.

Please Log in or Create an account to join the conversation.

Time to create page: 0.056 seconds
Powered by Kunena Forum