Bug in ps_session->url()

[doorknob]

One of the things that the ps_session->url() function does is to search the menu table for an entry that matches the set of parameters for a url so that the correct value for Itemid can be assigned. There are 2 problems with the way it works.

1 The queries use ambiguous selection criteria. "like 'category_id=1' would match any value that starts with 1, i.e. 1, 10, 11 etc.

2 The logic uses a cascade so that if a product_id is present but doesn't find a match (almost always the case, who has menu items for individual products?) the program then tries to match the category_id. If both values are present, the category menu entry is matched with a product - clearly wrong.

In my system I fixed this by replacing ps_session lines 513 to 544 (build 1760)

Code: [Select]

// Check if there is a menuitem for a product_id (highest priority)
if (!empty($ii_arr['product_id'])) {
if ($ii_product_id=intval($ii_arr['product_id'])) {
$db->query( "SELECT id FROM #__menu WHERE link='index.php?option=com_virtuemart' AND params like '%product_id=$ii_product_id%' AND published=1");
if( $db->next_record() ) $tmp_Itemid = $db->f("id");
}
}
// Check if there is a menuitem for a category_id
// This only checks for the exact category ID, it might be good to check for parents also. But at the moment, this would produce a lot of queries
if (!empty($ii_arr['category_id'])) {
$ii_cat_id=intval($ii_arr['category_id']);
if ( $ii_cat_id && $tmp_Itemid=='') {
$db->query( "SELECT id FROM #__menu WHERE link='index.php?option=com_virtuemart' AND params like '%category_id=$ii_cat_id%' AND published=1");
if( $db->next_record() ) $tmp_Itemid = $db->f("id");
}
}
// Check if there is a menuitem for a flypage
if (!empty($ii_arr['flypage'])) {
$ii_flypage=$db->getEscaped(vmget($ii_arr,'flypage'));
if ($ii_flypage && $tmp_Itemid=='') {
$db->query( "SELECT id FROM #__menu WHERE link='index.php?option=com_virtuemart' AND params like '%flypage=$ii_flypage%' AND published=1");
if( $db->next_record() ) $tmp_Itemid = $db->f("id");
}
}
// Check if there is a menuitem for a page
if (!empty($ii_arr['page'])) {
$ii_page=$db->getEscaped(vmget($ii_arr,'page' ));
if ($ii_page && $tmp_Itemid=='') {
$db->query( "SELECT id FROM #__menu WHERE link='index.php?option=com_virtuemart' AND params like '%page=$ii_page%' AND published=1");
if( $db->next_record() ) $tmp_Itemid = $db->f("id");
}
}
with

Code: [Select]

// Check for a matching menu item
if( $ii_arr['product_id'] ) { // Product page
$ii_product_id = (int)$ii_arr['product_id'];
$ii_cat_id = (int)$ii_arr['category_id'];
$ii_cat_id_required = false;
$ii_page = 'shop.product_details';
$ii_page_required = false;
if( $ii_arr['page'] ) {
$ii_page = $db->getEscaped( vmget($ii_arr, 'page') );
if( $ii_page != 'shop.product_details' ) {
$ii_page_required = true;
}
}
$ii_flypage = FLYPAGE;
$ii_flypage_required = false;
if( $ii_arr['flypage'] ) {
$ii_flypage = $db->getEscaped( vmget($ii_arr, 'flypage') );
if( $ii_flypage != FLYPAGE ) {
$ii_flypage_required = true;
}
}
} else if( !empty($ii_arr['category_id']) ) { // Category page
$ii_product_id = '';
$ii_cat_id = (int)$ii_arr['category_id'];
$ii_cat_id_required = true;
$ii_page = 'shop.browse';
$ii_page_required = false;
if( $ii_arr['page'] ) {
$ii_page = $db->getEscaped( vmget($ii_arr, 'page') );
if( $ii_page != 'shop.browse' ) {
$ii_page_required = true;
}
}
$ii_flypage = FLYPAGE;
$ii_flypage_required = false;
if( $ii_arr['flypage'] ) {
$ii_flypage = $db->getEscaped( vmget($ii_arr, 'flypage') );
if( $ii_flypage != FLYPAGE ) {
$ii_flypage_required = true;
}
}
} else if( !empty($ii_arr['page']) ) { // Non-Product/Non-Category page
$ii_product_id = '';
$ii_cat_id = '';
$ii_cat_id_required = false;
$ii_page = $ii_arr['page'];
$ii_page_required = true;
$ii_flypage = '';
$ii_flypage_required = false;
} else { // Default to finding the VM home page only
$ii_product_id = '';
$ii_cat_id = '';
$ii_cat_id_required = false;
$ii_page = 'shop.index';
$ii_page_required = true;
$ii_flypage = '';
$ii_flypage_required = false;
}
// Build the query
$q  = "SELECT id FROM #__menu WHERE link='index.php?option=com_virtuemart' AND published=1";
$q .= " AND params LIKE '%product_id=$ii_product_id\\n%'"; // Required even if $ii_product_id is empty
$q .= " AND (params LIKE '%category_id=$ii_cat_id\\n%'"; // Required even if $ii_cat_id is empty
$q .= $ii_cat_id_required ? ")" : " OR params LIKE '%category_id=\\n%')";
$q .= " AND (params LIKE '%page=$ii_page\\n%'"; // Required even if $ii_page is empty
$q .= $ii_page_required ? ")" : " OR params LIKE '%page=\\n%')";
$q .= " AND (params LIKE '%flypage=$ii_flypage\\n%'"; // Required even if $ii_flypage is empty
$q .= $ii_flypage_required ? ")" : " OR params LIKE '%flypage=\\n%')";

$db->query( $q );
if( $db->next_record() ) {
$tmp_Itemid = $db->f("id");
}

This, with the changes in other recent posts now appears to fix all of my breadcrumbs problems

Regards
Phil

[jjj2]

Thanks for this.

But if you implement such way, then the email to friend link will not only show the partial link to the product.

Eg.
...You can find it here:
/products/hangerseries/xtraseries/details/46/12pcs-kid-hanger

Which makes not much sense.

Is a full URL fix available ?

[doorknob]

But if you implement such way, then the email to friend link will not only show the partial link to the product.

I don't understand your point. What this function does is to improve the way that vm detects whether the url matches a menu item.

[aravot]

ps_session has been updated in rev. 1814, please check if it works or if any further changes is needed.

[doorknob]

ps_session has been updated in rev. 1814, please check if it works or if any further changes is needed.

Peter
I've updated to build 1814 and the the above patch is still required for the reasons already stated and also because the proposed approach uses fewer queries (every little helps) and facilitates the addition of new parameters to the vm menu type (which was one of the reasons I made the change in my system).

The version of the code posted above was edited to remove the code used for processing additional menu item parameters but post the code again to include that code because somebody is bound to want to add more parameters

Code: [Select]

// Check for a matching menu item
if( $ii_arr['product_id'] ) { // Product page
$ii_product_id = (int)$ii_arr['product_id'];
$ii_cat_id = (int)$ii_arr['category_id'];
$ii_cat_id_required = false;
$ii_page = 'shop.product_details';
$ii_page_required = false;
if( $ii_arr['page'] ) {
$ii_page = $db->getEscaped( vmget($ii_arr, 'page') );
if( $ii_page != 'shop.product_details' ) {
$ii_page_required = true;
}
}
$ii_flypage = FLYPAGE;
$ii_flypage_required = false;
if( $ii_arr['flypage'] ) {
$ii_flypage = $db->getEscaped( vmget($ii_arr, 'flypage') );
if( $ii_flypage != FLYPAGE ) {
$ii_flypage_required = true;
}
}
} else if( !empty($ii_arr['category_id']) ) { // Category page
$ii_product_id = '';
$ii_cat_id = (int)$ii_arr['category_id'];
$ii_cat_id_required = true;
$ii_page = 'shop.browse';
$ii_page_required = false;
if( $ii_arr['page'] ) {
$ii_page = $db->getEscaped( vmget($ii_arr, 'page') );
if( $ii_page != 'shop.browse' ) {
$ii_page_required = true;
}
}
$ii_flypage = FLYPAGE;
$ii_flypage_required = false;
if( $ii_arr['flypage'] ) {
$ii_flypage = $db->getEscaped( vmget($ii_arr, 'flypage') );
if( $ii_flypage != FLYPAGE ) {
$ii_flypage_required = true;
}
}
} else if( !empty($ii_arr['page']) ) { // Non-Product/Non-Category page
$ii_product_id = '';
$ii_cat_id = '';
$ii_cat_id_required = false;
$ii_page = $ii_arr['page'];
$ii_page_required = true;
$ii_flypage = '';
$ii_flypage_required = false;
} else { // Default to finding the VM home page only
$ii_product_id = '';
$ii_cat_id = '';
$ii_cat_id_required = false;
$ii_page = 'shop.index';
$ii_page_required = true;
$ii_flypage = '';
$ii_flypage_required = false;
}
// Build the query
$q  = "SELECT id FROM #__menu WHERE link='index.php?option=com_virtuemart' AND published=1";
$q .= " AND params LIKE '%product_id=$ii_product_id\\n%'"; // Required even if $ii_product_id is empty
$q .= " AND (params LIKE '%category_id=$ii_cat_id\\n%'"; // Required even if $ii_cat_id is empty
$q .= $ii_cat_id_required ? ")" : " OR params LIKE '%category_id=\\n%')";
$q .= " AND (params LIKE '%page=$ii_page\\n%'"; // Required even if $ii_page is empty
$q .= $ii_page_required ? ")" : " OR params LIKE '%page=\\n%')";
$q .= " AND (params LIKE '%flypage=$ii_flypage\\n%'"; // Required even if $ii_flypage is empty
$q .= $ii_flypage_required ? ")" : " OR params LIKE '%flypage=\\n%')";

// Additional parameters for EfaB
$param_names = array(
'orderby',
'discounted',
'featured'
);
foreach( $param_names as $param_name ) {
$ii_value = $db->getEscaped( vmget($ii_arr, $param_name) );
// Note: This syntax allows new parameters to be added by updating the array
$q .= " AND (params LIKE '%$param_name=$ii_value\\n%' OR params NOT LIKE '%$param_name=')";
}

$db->query( $q );
if( $db->next_record() ) {
$tmp_Itemid = $db->f("id");
}
This replaces lines 513 - 544.

Another change that is required is to fix to problem with the function getShopItemid() which only works properly if the menu item for the vm home page is the first vm menu item in the menu table because the selection criteria used in the query are too vague.
Line 463 should be replaced with

Code: [Select]

$db->query( "SELECT id FROM #__menu WHERE link='index.php?option=com_virtuemart' AND published=1 AND params LIKE '%page=shop.index\\n%'");
to ensure that the default Itemid value is the correct one.

Other issues with build 1814 that I noticed were
classes\class.img2thumb.php - thumbnail size calculation is still wrong.
line 191 should be changed from

Code: [Select]

if ($orig_size[0]<$orig_size[1])
to

Code: [Select]

if (($orig_size[0]*$maxY)<($orig_size[1]*$maxX))

mainframe.class.php - duplicates the breadcrumbs for items with their own menu item
line 391 should be changed from

Code: [Select]

global $mainframe;
to something like

Code: [Select]

global $mainframe, $sess;

// Only add the extra pathway if the menu item is using the default VM Itemid
$Itemid = $_REQUEST['Itemid'];
if( $Itemid && $Itemid != $sess->getShopItemid() ) {
return;
}

regards
Phil

[aravot]

Thanks Phil,

I am currently testing
Bug in class.img2thumb.php http://forum.virtuemart.net/index.php?topic=55201.0
was able to duplicate will commit the fix.

Bug in template.class.php http://forum.virtuemart.net/index.php?topic=54394.0
Trying to duplicate this.

Next will try breadcrumb issues, I tired but couldn't duplicate maybe because I use VM menu module and not direct URL link.

[jjj2]

But if you implement such way, then the email to friend link will not only show the partial link to the product.

I don't understand your point. What this function does is to improve the way that vm detects whether the url matches a menu item.

Hi..I tried your solution, but when I clicked on the top of each product, the email product link, it seems like somehow stripped off the full domain path, instead jus showing something like this :

/products/hangerseries/xtraseries/details/46/12pcs-kid-hanger

Do you have this problem ?

[doorknob]

I haven't tested the email link. I switched that off a long time ago because it didn't work.
Phil