"/ajax.php" adds itself to the navigation history

[lhungil]

Steps to Replicate:

1. Add an item to the shopping cart.
2. Visit shopping cart page.
3. At this point "Back to Shopping" returns the URL to the last page.
4. Refresh the shopping cart page (or update the quantity of a product).
5. At this point "Back to Shopping" no longer returns the URL to the last page.

It appears when "/ajax.php" is loaded via JavaScript, the contents of the navigation history are being updated to add "/index.php?main_page=index&act=ajaxPayment&method=setNoscriptCookie". While not a high priority, this may negatively affect the user experience.


Perhaps not the correct fix, but adding the following line to "/ajax.php" may be a workaround:

require ('includes/application_top.php');
$_SESSION['navigation']->remove_current_page();  // Remove page from navigation history
$language_page_directory = DIR_WS_LANGUAGES.$_SESSION['language'].'/';

[Ajeh]

See if this works better:

$_SESSION['navigation']->set_snapshot();

[Ajeh]

NOTE: this appears to work when coming from product_info page but more work is needed for when coming from the Product Listing ...

[lhungil]

When changing from "$_SESSION['navigation']->remove_current_page();" to "$_SESSION['navigation']->set_snapshot();" the issue is once again present.

Currently testing under PHP 5.4 and PHP 5.5.

[Ajeh]

Interesting ... it does fail ...

What worked was testing your method then changing the code with what I posted ... but what I posted does not work on a clean session ...

[DrByte]

Can someone test whether this works reliably?

/includes/classes/navigation_history.php
Around line 32 add the highlighted lines:

  function add_current_page() {


    // don't put the ajax handler calls into nav history
    if (preg_match('|ajax\.php$|', $_SERVER['SCRIPT_NAME']) && isset($_GET['act']) && $_GET['act'] != '') return;


    global $request_type, $cPath;

And also test on a Windows server too?

[lat9]

Can someone test whether this works reliably?

/includes/classes/navigation.php
At line 80 add the highlighted lines:

    // don't put the ajax handler calls into nav history
    if (preg_match('|/ajax.php$|', $_SERVER['SCRIPT_NAME']) && isset($_GET['act']) && $_GET['act'] != '') $set = 'false';


    if ($set == 'true') {
      if ($_GET['main_page']) {

And also test on a Windows server too?

Apologies, but what was wrong with lhungil's initial proposal? That implementation allows the ajax.php module to "take care of" itself, rather than muddying-up the navigation class.

[lhungil]

I kind of like the page / script itself handling itself... But I suspect there is a possibility for a race condition when someone has multiple tabs open (actually loading)... Potentially causing the wrong page to be removed (with the code I posted).

Perhaps a long term solution would be to change the code where a page is added to the navigation? Perhaps only adding the page to the history inside application_bottom (instead of application_top). Then we could set a "flag" in the page / script indicating it should not be added to the navigation history?

I've not dug into the core code far enough to see if this is already the way things work, but if not already implemented as above... I would recommend something along those lines...

Can someone test whether this works reliably?

/includes/classes/navigation.php
At line 80 add the highlighted lines:

    // don't put the ajax handler calls into nav history
    if (preg_match('|/ajax.php$|', $_SERVER['SCRIPT_NAME']) && isset($_GET['act']) && $_GET['act'] != '') $set = 'false';


    if ($set == 'true') {
      if ($_GET['main_page']) {

...

Appears to work for me under Windows x64 (PHP 5.4, 5.5, and 5.6) and Linux (Debian 7 - PHP 5.4.36).

[DrByte]

Apologies, but what was wrong with lhungil's initial proposal? That implementation allows the ajax.php module to "take care of" itself, rather than muddying-up the navigation class.

Simple: it didn't work properly.

[DrByte]

Perhaps a long term solution would be to change the code where a page is added to the navigation?

That's basically what my proposed change does.

I agree that a better solution is desirable, but the goal of any fix to share for v1.5.4 needs to be as brief and simple-to-implement as possible. Any broader architectural changes would go into a new version. I know you understand this. I'm just writing it for the sake of others who read this discussion.

At present the class is instantiated via the auto-loader system, and there's no interception option there to say "do or don't do based on XYZ conditions".


I personally never use those "go back to the previous page" navigation buttons, because I always just use the browser's "back" button ... it's a lot faster than having to find something to click on. If I had my way I'd remove all such buttons from all templates, as I feel they just clutter the page. But, I know some people swear by them and don't even realize that the browser has a "back" button. Sigh.