VirtueMart Forum

VirtueMart Dev/Coding Central: VM1 (old version) => Virtuemart 1.1 Development (Archiv) => Quality & Testing VirtueMart 1.1.x => Topic started by: doorknob on June 07, 2009, 04:13:52 AM

Title: Bug: Duplicated breadcrumbs
Post by: doorknob on June 07, 2009, 04:13:52 AM
VM adds text to the standard Joomla breadcrumbs to create a bespoke vm breadcrumb set. This works fine when the vm module is used to simulate a menu because vm starts with the breadcrumbs for its own 'home' page and then adds the product category and product name strings to create the final breadcrumbs. This breaks down if 'VirtueMart' menu items are used. In this situation, VM uses the actual id of the menu item as the start point for the breadcrumbs and then adds more strings that duplicate the values created by Joomla. I have fixed this by changing mainframe.class (line 391 in build 1760) from:
global $mainframe;

to:
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
Title: Re: Bug: Duplicated breadcrumbs
Post by: chrism on June 15, 2009, 14:15:16 PM
Phil,

Thank you, i have implement the change above and it has fixed my duplicate breadcrumbs issue.

My change was on line 355 as i say it works so it must be the same...

\administrator\components\com_virtuemart\classes\mainframe.class line 355 directly under
function vmAppendPathway( $pathway ).

Thanks again, very good work (which should go into the next release)
Chris
Title: Re: Bug: Duplicated breadcrumbs
Post by: chrism on June 15, 2009, 14:43:16 PM
Phil,

I notice now that when in an items flypage, the item itself is no longer in the breadcrumb and the category which is now the last breadcrumb has no link. I would like it the item still to be present and a link back to the category, is this possible? I am still very appreciative of the fix ;-)

Thanks
Chris
Title: Re: Bug: Duplicated breadcrumbs
Post by: doorknob on June 15, 2009, 18:45:24 PM
Chris,
The principle behind my mods to breadcrumbs is:
1 Use the Joomla default approach when the page corresponds to a menu item (i.e. the item has its own menu item and the url does not include 'Itemid=xx' where xx is the item id of the VM 'home' page)
2 Use the VM function to generate the breadcrumbs when the page is not mapped directly to a menu item. Usually, product detail pages don't have menu items (not on my site).

If the product detail pages have breadcrumbs that don't include the product name then the page is considered to have its own Itemid and the Joomla breadcrumbs are being displayed instead of the VM breadcrumbs. To get the vm breadcrumbs, the page must be recognised as not having its own menu item. Take a look at this post http://forum.virtuemart.net/index.php?topic=56531.0 (http://forum.virtuemart.net/index.php?topic=56531.0).
Regards
Phil
Title: Re: Bug: Duplicated breadcrumbs
Post by: chrism on June 15, 2009, 22:38:34 PM
Phil,

Thanks again your previous post explains why i am seeing what i'm seeing, i have previously implemented http://forum.virtuemart.net/index.php?topic=42931.msg162939#msg162939
which enabled me to keep focus on my VM menu items even when displaying a details page, i did this so that i could control the modules that are displayed in individual categories/flypages. Keep up the good work, i appreciate, and i'm sure many others will appreciate your work.

Thanks
Chris
Title: Re: Bug: Duplicated breadcrumbs
Post by: vmfan on August 02, 2009, 10:08:22 AM
Quote from: doorknob on June 07, 2009, 04:13:52 AM
VM adds text to the standard Joomla breadcrumbs to create a bespoke vm breadcrumb set. This works fine when the vm module is used to simulate a menu because vm starts with the breadcrumbs for its own 'home' page and then adds the product category and product name strings to create the final breadcrumbs. This breaks down if 'VirtueMart' menu items are used. In this situation, VM uses the actual id of the menu item as the start point for the breadcrumbs and then adds more strings that duplicate the values created by Joomla. I have fixed this by changing mainframe.class (line 391 in build 1760) from:
global $mainframe;

to:
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

Thanks, thanks, thanks, thanks, 

THOUSAND THANKS

I searched so long to a solution....


Exuse my English...it´s not so good ....

Regards
Brigitte
Title: Re: Bug: Duplicated breadcrumbs
Post by: dylanjh on September 18, 2009, 16:33:42 PM
Hmmmmmmph! When I make the change it tells me "PHP Fatal error:  Call to a member function getShopItemid() on a non-object in /var/www/html/sigma/administrator/components/com_virtuemart/classes/mainframe.class.php on line 358"

Any ideas people?
Title: Re: Bug: Duplicated breadcrumbs
Post by: doorknob on September 22, 2009, 01:49:17 AM
Looks like you didn't declare $sess as a global or mistyped the variable name.

getShopItemid is a method of the $sess object. This object is created by VM as part of the standard VM environment and declared as a global variable. $sess therefore already exists but to use it you must make sure that it has been declared as a global in the function where it is to be used.

Good luck
Phil
Title: Re: Bug: Duplicated breadcrumbs
Post by: Forrest on December 29, 2009, 07:41:08 AM
Quote// Only add the extra pathway if the menu item is using the default VM Itemid
      $Itemid = $_REQUEST['Itemid'];
      if( $Itemid && $Itemid != $sess->getShopItemid() ) {
         return;
      }

Yes, right above this add:
global $sess;

So the final should be:

// Only add the extra pathway if the menu item is using the default VM Itemid
      $global $sess;
      $Itemid = $_REQUEST['Itemid'];
      if( $Itemid && $Itemid != $sess->getShopItemid() ) {
         return;
      }
Title: Re: Bug: Duplicated breadcrumbs
Post by: doorknob on December 29, 2009, 11:22:26 AM
QuoteSo the final should be:

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

The original post does include this (added into the existing global statement)
Title: Re: Bug: Duplicated breadcrumbs
Post by: Forrest on December 30, 2009, 09:54:52 AM
Hmmm, late night I guess. Not as smart as a doorknob :( Thanks.
Title: Re: Bug: Duplicated breadcrumbs
Post by: randomperson on September 21, 2010, 01:38:04 AM
is this going to get fixed in the official release or something...?

by the way, this `fix` creates a new issue, when browsing for the product itself it won't display breadcrumb for it at all (when there is a joomla menu assigned to the specific category)
Title: Re: Bug: Duplicated breadcrumbs
Post by: doorknob on September 24, 2010, 00:30:46 AM
Quoteby the way, this `fix` creates a new issue, when browsing for the product itself it won't display breadcrumb for it at all (when there is a joomla menu assigned to the specific category)
Works OK on my site. Take a look here http://www.eyeforabargain.co.uk/Bargains-Bazaar/Newfix-Car-Seat-Panama.html (http://www.eyeforabargain.co.uk/Bargains-Bazaar/Newfix-Car-Seat-Panama.html) for example.
Title: Re: Bug: Duplicated breadcrumbs
Post by: kelsay on September 28, 2010, 20:40:57 PM
Hi doorknob!  Sorry to intrude on this thread...

I too have a problem with my breadcrumbs not being okay.
They are fine for most pages and products, except the My Account and CART links are not.
I get HOME -> CATEGORY -> CART while it should say  HOME -> CART.

I have tried to find " $Itemid = $_REQUEST['Itemid'];" in my mainframe.class.php but it is not there. I'm using 1.5.20 and 1.1.15.

Can you help me with this?



Title: Re: Bug: Duplicated breadcrumbs
Post by: doorknob on October 03, 2010, 00:43:07 AM
QuoteI have tried to find " $Itemid = $_REQUEST['Itemid'];" in my mainframe.class.php but it is not there
The mod I posted was designed to change the way that the breadcrumbs are generated when a page is linked to a menu item. VM links every page to its own home page by using the $sess->getShopItemid() function (although that function has a bug because the sql it uses is not sufficiently selective in all cases). If, like me, you want to be able to use the Joomla security and configuration features that are associated with menu items, rather than treay the whole of VM as a single page then it is necessary to make the change I posted. The "$Itemid = $_REQUEST['Itemid'];" code identifies the itemid associated with the menu item that called the current page and is not the way that VM usually works.

I don't use the cart and so I can't replicate your issue. Unless your cart is linked to a menu item then I would not expect this patch to be responsible. If it is linked to a menu item, I suggest that you include Itemid=nn in the menu item url, where nn is the itemid of the VM home page (find the id of your main vm menu item in the menu editing area). If you are using sef, don't forget to clear the cache.
Regards
Phil
Title: Re: Bug: Duplicated breadcrumbs
Post by: kelsay on October 03, 2010, 20:11:08 PM
Thank you for your input!
Title: Re: Bug: Duplicated breadcrumbs
Post by: zanardi on October 17, 2010, 22:31:05 PM
Well, i discovered this thread AFTER i submit an official FIX in SVN. My fix was to check if we are in a menu item directly linked to a product id or category id, and in that case NOT to add VM pathway. I guess it could solve the duplicate item issue while at the same time keeping the pathway everywhere its needed.

For discussions and/or critics, my code is:


//retrieve menu parameters
$menus = &JSite::getMenu();
$menu  = $menus->getActive();
$menu_params = new JParameter( $menu->params );
$menu_product_id = $menu_params->get('product_id', 0);
$menu_category_id = $menu_params->get('category_id', 0);

// retrieve current VM element id
$vm_product_id = vmRequest::getVar('product_id', 0);
$vm_category_id = vmRequest::getVar('category_id', 0);

// avoid double pathway
if ( ( $menu_product_id != 0 && $menu_product_id == $vm_product_id ) or ( $menu_category_id != 0 && $menu_category_id == $vm_category_id ) ) {
return;
}


Title: Re: Bug: Duplicated breadcrumbs
Post by: kelsay on October 18, 2010, 07:05:00 AM
@Zandardi

Can you please tell me where this code needs to go?  Which file?
Title: Re: Bug: Duplicated breadcrumbs
Post by: zanardi on October 18, 2010, 10:37:05 AM
@kelsay:
this code, which still needs some testing, goes in the file mainframe.class.php, in the function vmAppendPathway. I report here the whole new function for clarity:


function vmAppendPathway( $pathway ) {

global $mainframe;

//retrieve menu parameters
$menus = &JSite::getMenu();
$menu  = $menus->getActive();
$menu_params = new JParameter( $menu->params );
$menu_product_id = $menu_params->get('product_id', 0);
$menu_category_id = $menu_params->get('category_id', 0);

// retrieve current VM element id
$vm_product_id = vmRequest::getVar('product_id', 0);
$vm_category_id = vmRequest::getVar('category_id', 0);

// avoid double pathway
if ( ( $menu_product_id != 0 && $menu_product_id == $vm_product_id ) or ( $menu_category_id != 0 && $menu_category_id == $vm_category_id ) ) {
return;
}

// Remove the link on the last pathway item
$pathway[ count($pathway) - 1 ]->link = '';

if( vmIsJoomla('1.5') ) {
$cmsPathway =& $mainframe->getPathway();
foreach( $pathway AS $item) {
$item->link = str_replace('&', '&', $item->link);
// make sure that ' (apostrophe) is converted to '
$item->name = html_entity_decode( $item->name, ENT_QUOTES );
$cmsPathway->addItem( $item->name, $item->link );
}
} else {
$tpl = vmTemplate::getInstance();
$tpl->set( 'pathway', $pathway );
$vmPathway = $tpl->fetch( 'common/pathway.tpl.php' );
$mainframe->appendPathWay( $vmPathway );
}
}
Title: Re: Bug: Duplicated breadcrumbs
Post by: franzpeter on October 18, 2010, 14:23:38 PM
Did implement the code into my page (VM 1.1.5). The double breadcrumbs did disappear but the last entry for the product too. In the VM demo the breadcrumb shows: Home > Hand Tools > Hammer but with the patch class it shows only:

Home > Hand Tools (without Hammer as last entry). So the question is:
Should the last entry i.e. the product name appear or should it not appear in the breadcrumb trail?

Title: Re: Bug: Duplicated breadcrumbs
Post by: zanardi on October 18, 2010, 16:52:42 PM
Well, i am not here to say what's right and what's wrong :-)

Obviously this should be fixed, my code should only be invoked if we ARE in a menu item which HAS a product id or category id parameter specified; and it should not change default behaviour. Some more testing is needed... i'll let you know as soon as i have news.
Title: Re: Bug: Duplicated breadcrumbs
Post by: doorknob on October 20, 2010, 22:13:50 PM
QuoteShould the last entry i.e. the product name appear or should it not appear in the breadcrumb trail?
Today is the first time that I've looked at Zanardi's patch in detail. My original patch and Zanardi's alternative version are both trying to identify where allowing VirtueMart to add to the standard Joomla breadcrumbs is not desirable. Joomla creates breadcrumbs based on the menu structure (which it works out from the current value of Itemid, working backwards through the menu structure). The VM approach to menus is based on the principle that the category menu is provided by a module (which is not really a menu in the Joomla sense). A standard VM implementation has just one menu item (the home page). That's why a range of Joomla control and security features don't work in VM like they do in other components. When VM adds extra items to the breadcrumbs, it is because it is expecting the stub created by Joomla to correspond to just the home page only. VM then expects to add the items that correspond with it's own dummy menu. Those of us that reject the mickey mouse menu are stuck with having to cope with VM assuming that we are using it. My original patch works by checking whether the menu item had a different id from the home page. This approach allows the VM menu module to work in its quaint way but for regular menu items to be treated in the same way as non-VM items. This may be a problem for some pages. There are some pages where I want to add an extra item to the breadcrumbs (e.g. for speshitpillt cross-category searches such 'featured products' or 'on special'). For those, I added a second parameter to the vmAppendPathway() function to permit me to force an extra entry to the breadcrumbs when needed. My site doesn't use the cart and so there may be pages in that area that need special treatment. The alternative patch provided by Zanardi takes a different approach. Instead of a simple generic way to identify the pages where the extra breadcrumbs are not needed, it tries to positively identify those pages using quite complex logic. This approach is always going to need more and more complexity to cope with exceptions. For a start, it can't handle a manufacturer based browse. Looking at the detail, it seems to have other problems
$menu_product_id != 0 && $menu_product_id == $vm_product_idThis identifies menu items for a specific product and suppresses VM breadcrumbs. For products not linked to the current menu item, the breadcrumbs will have the full VM category structure and the product name added. It's not clear to me why this would be desirable.
$menu_category_id != 0 && $menu_category_id == $vm_category_idPresumably this is intended to apply to browse pages for categories but will also usually apply to product detail pages (which may be unintended). When menu items for specific products are correctly configured, they should have both a product_id and a category_id value. Even if that were not the case, the logic should allow for the possibility of users setting them up that way. To prevent this applying to product detail pages it would be necessary to add '&& $menu_product_id == 0'.
A better approach is to keep it simple and use the original patch. If the patch causes truncation of the breadcrumb for specific pages then it would be better to test the value of $page and allow exceptions on that basis. The original patch takes the premise that Joomla gets the breadcrumbs right and VM messes it up. The alternative patch takes the opposite view that VM's interference in the breadcrumbs is acceptable except under specific circumstances. You choose.
Title: Re: Bug: Duplicated breadcrumbs
Post by: zanardi on October 21, 2010, 10:20:10 AM
Most of what you say is correct and your technical analysis is good. The addition to category_id check is a correct suggestion.

But I never meant to change the way VM produces is pathway. I started from a much simpler request: remove VM appended pathway whenever i manually create a menu item directly related to a specific product or category.

Example.

I have a shop which has a "BIKES" category and a "THUNDER" product. This is my menu:

HOME
SHOP (menu type: VirtueMart, no parameters)

I'll click on shop, pathway is Home -> Shop
I see categories listing, i click on BIKES, patwhay is Home -> Shop -> Bikes
I see product listing, i click on THUNDER, patwhay is Home -> Shop -> Bikes -> Thunder

Now i create another couple menu items, and my menu becomes:

HOME
SHOP   (menu type: VirtueMart, no parameters)
BIKES   (menu type: VirtueMart, category_id = 4)
THUNDER   (menu type: VirtueMart, product_id = 12)

With VM 1.1.5:
- if i click on BIKES menu item, pathway is Home -> Shop -> Bikes -> Bikes
- if i click on THUNDER menu item, pathway is Home -> Shop -> Thunder -> Bikes -> Thunder

This is what i am trying to address, because many people asked to do that. I'll stick with that for 1.1.6, with all aforementioned corrections.




Title: Re: Bug: Duplicated breadcrumbs
Post by: doorknob on October 22, 2010, 00:21:26 AM
You haven't said why the more generic solution is not satisfactory
Title: Re: Bug: Duplicated breadcrumbs
Post by: aravot on October 22, 2010, 02:06:52 AM
The following post http://forum.virtuemart.net/index.php?topic=77191.0 has a video of breadcrumb issue.
Title: Re: Bug: Duplicated breadcrumbs
Post by: doorknob on October 22, 2010, 14:02:35 PM
All that video does is support the point I made that a generic solution such as the one I use will be more robust. I've been using that solution since the original post on this thread was made and it works in every situation that I've found including navigating to next and prior products from a detail page. Making the logic more complex is fraught with dangers. When I looked at what you proposed, I could see that manufacturer_id was not covered, the logic also assumed that on a detail page the category_id is zero. Going that way just digs a bigger and bigger hole.
Title: Re: Bug: Duplicated breadcrumbs
Post by: zanardi on October 22, 2010, 16:10:54 PM
@aravot: thank you, but we have no problem understanding what the issue is: we are now discussing what's the best solution, so your video is mis-timed.

@doorknob: you are answering to Peter Osipof (aravot), not to me. I am Francesco Abeni (zanardi) and i was going to answer that your solution does in fact look more solid. As i said, i discovered this thread AFTER i made my modifications; i was going to have another look at it soon and i think i am going to use what you propose.

At my first look, i guessed that you were solving a different problem; but i can see i was wrong. So be happy and peaceful: i WILL use your solution and there's no need to argue anymore :-)
Title: Re: Bug: Duplicated breadcrumbs
Post by: aravot on October 22, 2010, 17:46:33 PM
I know you understand the issue, I posted the video for those users who view the topic to understand it better, visual is always better.
Title: Re: Bug: Duplicated breadcrumbs
Post by: doorknob on October 22, 2010, 18:25:49 PM
Quotei was going to have another look at it soon and i think i am going to use what you propose.
You might also need to look at the getShopItemid function because the 'where' clause is so vague that it just picks the first vm menu item that it finds that is published. Usually that's ok because the 'home' page is the first one that was set up but it won't be for everybody. I made a separate post about that some time ago (suggested adding "AND params LIKE '%page=shop.index\\n%'" into the selection criteria).
Phil
Title: Re: Bug: Duplicated breadcrumbs
Post by: simply123 on May 26, 2011, 08:55:12 AM
great fix doorknob, simple and perfect
Title: Re: Bug: Duplicated breadcrumbs
Post by: andyn on July 14, 2011, 02:51:05 AM
Just wanted to confirm doorknob's solution works perfectly - running Joomla 1.5.23 and VM 1.1.4

Thanks