nexcess / magento-turpentine

A Varnish extension for Magento.
GNU General Public License v2.0
519 stars 253 forks source link

Compare Products side block is compromised! #895

Closed ADDISON74 closed 8 years ago

ADDISON74 commented 9 years ago

Sorry for re-posting again, but I found one more issue related to this annoying behavior:

1) CLEAR ALL button When you use this button it should clear the comparison list and refresh the page. It won't do this. After Magento cleans the list, you will be redirected to the last past you added a product to the comparison list. Please add a product to comparison list, visit a new category and click on Clear All button. The page loads not with the current category content but the page content you added the product for comparison (category or product page).

2) Deleting individual products in the list (X buttons) Same issue here too. When you delete a product from list your page content after refreshing will be the last one you added a product for comparison instead of current one.

Is this an AJAX related issue like in Community Poll?

As it is now, comparison is totally compromised. It can be used hoping the visitor won't clear products from list or the whole list. When a visitors clears the list he will become confused for sure trying to understand what happened. Please investigate and provide a solution asap. Initially I was disabling Clear All button by hiding it with CSS. Now all delete individual buttons have issues too, cannot hide them all.

Thank you.

miguelbalparda commented 9 years ago

We could really use some more informative titles for the issues you post, no one having the same issue will be able to find this with the current titles. Anyway we will check this and get back to you.

ADDISON74 commented 9 years ago

aricwatson investigated this issue a couple of weeks ago. but only for Clear All button. the initial issue report is closed. today I found an new issue related to #2 from above. it is only a matter of history/redirect after page reloads.

thank you.

aricwatson commented 9 years ago

Yes, I believe there is a larger issue related to referer problems which I hope to track down at some point. There are a number of issues where the user is redirected differently when when using Varnish than otherwise.

We're still tracking that down, I think for now we can close this particular issue.

ADDISON74 commented 9 years ago

More on this:

Case 1: 1) Go visit Category A, add Product 1 2) Go visit Category B, add Product 2 3) Go visit Category C, add Product 3 and STAY in this category.

Now delete individual products or Clear All, everything is working just perfect.

Case 2: 1) Go visit Category A, add Product 1 2) Go visit Category B, add Product 2 3) Go visit Category C, add Product 3 4) Go visit Category D

Now delete individual products or Clear All. You will be always redirected to the last page you added a product for comparison.

After "Case 2" steps 1) If you go visit again Category D, Clearing All and individual products, everything is working just 2) perfect. But if you go visiting Category E, again the issue appears

2) If you go visit a new Category E, Clearing All and individual products it redirect to Category D.

ADDISON74 commented 9 years ago

@aricwatson Please reopen this issue too. it should be marked as a bug.

Any positive thoughts after investigating it?

aricwatson commented 9 years ago

We're still looking into the issues surrounding redirects like this. It's an issue we're aware of and working on, and I think this particular issue is just one manifestation of the larger one. I'll reopen it to help us remember to check this particular scenario when we find a solution. Thanks for the very specific steps to replicate - always helpful!

ADDISON74 commented 9 years ago

Great. I appreciate for your dedicated work. I am not rushing you at all. This comparison issue is very annoying. I pretty sure you will find out what is going wrong. Turpentine extension made a lot of steps in good in the last months.

ADDISON74 commented 8 years ago

@thampe came with this possible solution: I had a similar issue. The problem is that magento encodes the redirect url inside the remove url which is cached by your varnish server. You can fix it if you build the compare url manually without the encoded url parameter:

// default $this->helper('catalog/product_compare')->getRemoveUrl($_item); // manually $this->getUrl('catalog/product_compare/remove', array('product' => $_item->getId()));

files to edit: app/design/{theme}/{package}/template/catalog/product/compare/sidebar.phtml app/design/{theme}/{package}/template/catalog/product/compare/list.phtml

Or override the helper Mage_Catalog_Helper_Product_Compare

ADDISON74 commented 8 years ago

I checked the files. In Magento 1.9.2.1 here are the content to modify:

File: list.phml

from

<td class="a-right"><a href="#" class="btn-remove" onclick="removeItem('<?php echo $this->helper('catalog/product_compare')->getRemoveUrl($_item) ?>');" title="<?php echo $this->quoteEscape($this->__('Remove This Item')) ?>"><?php echo $this->__('Remove This Item') ?></a></td>

to

 <td class="a-right"><a href="#" class="btn-remove" onclick="removeItem('<?php echo $this->getUrl('catalog/product_compare/remove', array('product' => $_item->getId())); ?>');" title="<?php echo $this->quoteEscape($this->__('Remove This Item')) ?>"><?php echo $this->__('Remove This Item') ?></a></td>

File: sidebar.html

from

<a href="<?php echo $_helper->getRemoveUrl($_item) ?>" title="<?php echo $this->quoteEscape($this->__('Remove This Item')) ?>" class="btn-remove" onclick="return confirm('<?php echo Mage::helper('core')->jsQuoteEscape($this->__('Are you sure you would like to remove this item from the compare products?')) ?>');"><?php echo $this->__('Remove This Item') ?></a>

to

<a href="<?php echo $this->getUrl('catalog/product_compare/remove', array('product' => $_item->getId())); ?>" title="<?php echo $this->quoteEscape($this->__('Remove This Item')) ?>" class="btn-remove" onclick="return confirm('<?php echo Mage::helper('core')->jsQuoteEscape($this->__('Are you sure you would like to remove this item from the compare products?')) ?>');"><?php echo $this->__('Remove This Item') ?></a>

It seems to be working. I will do more testing. @thampe please check if I posted right the modifications.

There is one more issue [Clear All] button which is still affected by redirecting. The method is $_helper->getClearListUrl()

After seeing the Turpentine needs to rewrite some methods related to comparison in order to fix this annoying redirection issue.

ADDISON74 commented 8 years ago

Is it necessary to change list.phtml file? Just changing only in sidebar.phtml it seems to fix the issue for removing individual products. There is no redirecting at all.

thampe commented 8 years ago

Hi @ADDISON74,

your modifications are correct. The way I suggested you to fix the issue, should not be implemented by the Turpentine Module itself, because it requires template changes, which will be in most cases overridden by a custom theme.

There are two other options to fix the redirect problem in general:

Both solutions can break some of magentos core functionality.

ADDISON74 commented 8 years ago

@thampe : Did you find a solution for [Clear All] button which is affected in the same way?

thampe commented 8 years ago
$this->getUrl('catalog/product_compare/clear');

this should do the trick ;)

ADDISON74 commented 8 years ago

Thank you. Your suggestion worked.

I hope Turpentine developers will take in consideration this option and find a way not editing our templates. It is a matter of redirecting at all.

ADDISON74 commented 8 years ago

any feedback or update from Turpentine developers related to this issue?

ADDISON74 commented 8 years ago

I will close this issue for opening other one with more details and extended.

ADDISON74 commented 8 years ago

Just to let you know what I found from #119:

@aheadley : "This should actually be fixed as of 0.5.2 . Turpentine will now generally redirect users back to the page they were on instead of to /checkout/cart/ for most cases."

Is still the way today in Turpentine? This is not good at all.