Bug #3634

[Workbench] Page content is the same as where you left it when navigating (back) to it with browser history

Added by Tom Clegg about 5 years ago. Updated almost 5 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Phil Hodgson
Category:
-
Target version:
Start date:
09/16/2014
Due date:
% Done:

100%

Estimated time:
(Total: 1.00 h)
Story points:
1.0

Description

Page content can be updated by Javascript and AJAX, but if it is, and one navigates away, and then uses the browser back button (or history) to return to the page, the user will see the initial content of the page (as it was previous to the JS and AJAX changes) rather than the most recent content of the page.

HTML5 allows us to store state in the browser's history stack.
  • Use replaceState() when changing page/tab content.
  • When switching tabs, use pushState() or replaceState() and store "which is the current tab" in the history data.
  • Listen to popstate events. Copy latest page content from the history data to the DOM.

Workbench already loads History.js, which offers compatibility with HTML4 browsers. https://github.com/browserstate/history.js


Subtasks

Task #3881: Review 3634-tab-stateResolvedPhil Hodgson

Task #4080: Deliver all pages with an expired cacheResolvedPhil Hodgson

Task #3880: Remember active tab panel on browser back navigationResolvedPhil Hodgson

Associated revisions

Revision 6b19642e
Added by Phil Hodgson almost 5 years ago

Merge branch '3634-tab-state' refs #3634

Revision a151775e (diff)
Added by Brett Smith almost 5 years ago

3634: Update user setup tests for preserved tab state.

These tests assume that the page reloads after submitting Ajax
dialogs. They started failing when we started preserving tab state.
Update the tests to expressly refresh the page. Future improvements
might find a solution with lower overhead.

Refs #3634.

History

#1 Updated by Tom Clegg almost 5 years ago

  • Description updated (diff)

#2 Updated by Ward Vandewege almost 5 years ago

  • Target version changed from Arvados Future Sprints to 2014-09-17 sprint

#3 Updated by Ward Vandewege almost 5 years ago

  • Assigned To set to Phil Hodgson

#4 Updated by Phil Hodgson almost 5 years ago

Well I did a bit of research and found a gem (https://github.com/mnarayan01/bootstrap-tab-history) that seems to handle this exactly as described. Why ask why not if it works?

#5 Updated by Radhika Chippada almost 5 years ago

Review feedback:

Either I do not understand the requirement or it is not working correctly.

- I went to my home project. It has 6 collections in the "Data collections" tab.
- I then went to a different project.
- I went to my db directly and deleted some of those collections that are part of the Home project.
- Now I used the back button to go back to my Home project and still see all 6 collections listed.
- Even after I went to a different tab, such as Jobs and pipelines, and went back to "Data collections" tab, I see the stale data.
- If I refresh the project (URL), then I see the right data.
- I repeated this several times, to confirm

Another test I performed:
- Go to collections page
- Combine some collections (this created a new collection and is now part of home project)
- Use back button to go back to Home project
- I do not see the new collection
- I see it after I refresh the URL

Adding this in case it helps debug the issue:
- Went to my Home project
- Clicked on a different tab (Jobs and pipelines)
- Now went to a different project
- Combined collections (into home project)
- Used back button and now I am in Jobs and pipelines tab
- Click on Data collections tab and it is empty
- The tab is loaded with data when I refresh using the project

Please let me know if I misinterpreted the requirement.

#6 Updated by Phil Hodgson almost 5 years ago

  • Status changed from New to In Progress

#7 Updated by Phil Hodgson almost 5 years ago

  • Target version changed from 2014-09-17 sprint to 2014-10-08 sprint

#8 Updated by Phil Hodgson almost 5 years ago

  • Subject changed from [Workbench] Auto-refreshed tabs revert to initial content if you navigate away and using browser Back button. to [Workbench] Page content is the same as where you left it when navigating (back) to it with browser history

#9 Updated by Phil Hodgson almost 5 years ago

  • Description updated (diff)

#10 Updated by Phil Hodgson almost 5 years ago

As noted above, my first iteration was to address only the original title of the Redmine issue, and I found a gem which did exactly that: it remembered the state of a Bootstrap tab when navigating back and forth.

But it was decided that the Redmine should actually have to do with the state of any page in all of Workbench: when using the browser back button, we expect to see what we last saw when we left the page.

It wasn't difficult to get it to remember all of the contents of <body> when unloading a page and then replacing <body> with the stored data onload.

What is a pain is dealing with the consequences of this happening. There is much of a web application that does not attempt to clean itself up before navigating away from a page. In general we don't feel this is necessary: we can show modals, spinners, whatever we want, and if we're navigating away it's no big deal because they'll all be discarded. But if we save the whole visual part of the DOM when we navigate away, and replace onload, we come back too literally to exactly what we left.

So the first thing I tried to do was identify these modals, spinners, etc., and just clean them up when using onload. I reckon this clean-up is finite and maintainable. I also reckon that we could refactor this to be a bit more "object oriented" and have it so that anything that can be shown on a page such as modals and spinners should also register itself to clean itself up after it is used. A modal should hide/close itself when a button on it causes navigation. A spinner registered to show on a certain event should register to hide itself after the processing is done, even if normally it ends in navigation. Etc.

window.addEventListener("beforeunload", function(e) {
    history.replaceState( { body: $('body').html() }, '' );
});

window.addEventListener("load", function(e) {
    if(history.state && history.state.body) {
        $('body').html( history.state.body );
        $('.tooltip').hide();
        $('.modal').hide();
        $('.modal-backdrop').remove();
        $('.loading').hide();
    }
});

But on testing the above with different parts of the application it quickly becomes apparent that it doesn't suffice even in some very simple use cases. An example:

  1. Show Project A collections: they are X, Y, and Z
  2. Show collection Z
  3. Move collection Z to Project B (you are then automatically navigated to Project B's collections)
  4. Hit browser back

We will be looking at collection Z in Project A, because that's how we left that page.

This is where this issue starts to become annoying, and where the different kinds of desired behaviour that revolve around it become conflicted. It does not seem desirable to me to show collection Z in Project A at this point. It would be better to have expired the cache. This, to the best of my research, can't be done after the fact. Nor can one go back through the whole history and rewrite it all (browser security issue). It's only generally possible to replace the state on the current page, or to add new states to the browser history.

I worked a bit on it, thinking that at least I can make it so that when the using clicked to move the collection to Project B that Workbench knows that if the user ever comes back to the page it should be reloaded.

I'll paste all the code here for this idea, including the basic functionality on unload and on load: I just added a new class to the move button: "force-cache-reload" and then in application.js the following jquery:

    $(document).on('click', '.force-cache-reload', function(e) {
        history.replaceState( { nocache: true }, '' );
    });

and javascript:

// Store the entire body of the page in the history
window.addEventListener("beforeunload", function(e) {
    if(!(history.state && (history.state.nocache || history.state.loading))) {
        history.replaceState( { body: $('body').html() }, '' );
    }
});

// When loading a page, check to see if there is a state recorded in the history.
// If it's the "right kind" (i.e. it's a hash with a "body" element), replace the current
// body with that in the history's state.
window.addEventListener("DOMContentLoaded", function(e) {
    if(history.state) {
        if(history.state.nocache) {
            $('body').html( '<h1>Wait....</h1>' );
            history.replaceState( { loading: true }, '' );
            location.reload(true);
        } else if(history.state.body) {
            $('body').html( history.state.body );
            $('.tooltip').hide();
            $('.modal').hide();
            $('.modal-backdrop').remove();
            $('.loading').hide();
        }
    }
});

(Note the use of DOMContentLoaded rather than load or popstate. This seemed after trial and error the best way to handle it.)

So the above works, if a bit clumsily. Obviously we'd want something nicer looking than my <h1>Wait...</h1> if we were to use something like this (perhaps even nothing at all would be better). But I soon started to see that this would only be a partial measure anyway, because if the user hits back a second time they are faced with the Project A collections, which will of course include Z! Again, to the best of my research, we could not use javascript to manipulate stuff already in the browser's past.

So now we'd have to start devising an actual data-specific caching system, where we keep track of what pages contain what objects, and invalidating a page's cache if they contain an object which itself has changed. To this I say: whoa!

Ultimately, my recommendation is simple: make none of Workbench cache itself in browsers. This can be done with a before_filter method like this:

  def set_cache_buster
    response.headers["Cache-Control"] = "no-cache, no-store, max-age=0, must-revalidate" 
    response.headers["Pragma"] = "no-cache" 
    response.headers["Expires"] = "Fri, 01 Jan 1990 00:00:00 GMT" 
  end

And so I would recommend implementing this either for all of Workbench or at least for every part whose content we're concerned about.

Then I would recommend, for fully addressing this original Redmine issue, that we use the HTML5 browser history stuff in conjunction with anchors in the URLs for the tabs, so that when the user hits browser back and they happen to be navigating back to a page with tabs that the correct tab is shown as the current one.

#11 Updated by Phil Hodgson almost 5 years ago

Note that the History.js magical plug-in seemed to be a total disaster when I tried to use it with any of this stuff. It really doesn't seem to work the same way as the HTML5 history, but maybe it would with a bit more futzing around. I'm not sure how this issue is going to be resolved, but I've already spent so much time on it that I didn't want to spend more, for the dubious benefits of compability with older browsers, without knowing what we'll do, since I'm inclined to recommend just never caching anything in the browser history, set every cache to immediately expire and have a page always reload when the user uses the browser back button.

#12 Updated by Phil Hodgson almost 5 years ago

After a meeting with Tom we agreed to settle on simplicity and so to set it up so that the cache is simply always expired; also to make it so that, if there are tab panels, the tab panel that the user had last viewed is the current one when using browser-back.

#13 Updated by Phil Hodgson almost 5 years ago

I've implemented a combination of compromises that work well according to my manual testing:

  • the bootstrap tab state gem does what it should: it remembers which tab was selected on which page if we go back to it with the browser
  • via the new set_cache_buster before_filter in ApplicationController the response headers will now always include directives to not cache; though this will not stop modern browsers from using their bfcache in certain cases, it will in most cases usefully force a reload of content on moving back in the browser history
  • there is already one known case where setting these response headers does not have the desired effect: as in the essay above, when showing a collection and then using the "move to project..." button, the browser will be navigated by javascript to the target project index page. Unfortunately, on browser-back the browser's bfcache takes over and the response headers are ignored. To resolve this, it is also now possible to add a "force-cache-reload" class to a link that would cause this problem, and then on browser-back a loading modal will be shown while the page is forcibly reloaded

I should add that while observing the browser-server communications with the above changes in place that, aside from the (resolved) problem described in the last bullet point above, there are many quite sensible cases where the directives to not cache are ignored by the browser and its bfcache is used. We should however be vigilant to spot cases where we could skip the new set_cache_buster before_filter, where we know that the content will never change.

#14 Updated by Phil Hodgson almost 5 years ago

Maybe though, if we keep the idea of forcing a reload via javascript under certain circumstances, then we should get rid of the modal I introduced and just use the usual AJAX spinner.

#15 Updated by Peter Amstutz almost 5 years ago

Since all of the workbench pages are dynamically generated, telling the browser not to cache the pages seems like a fine solution.

Returning to the same bootstrap tab when you go back is helpful.

If I understand this correctly, the "force-cache-reload" behavior class is needed for situations where the page content is being replaced through an AJAX call, rather than a normal reload, so disabling caching doesn't quite do the right thing in that situation?

Looks good to me, please merge.

#16 Updated by Phil Hodgson almost 5 years ago

re: force-cache-reload - yes, it's possibly some kind of javascript-browser interaction inconsistency, but it seems as if when window.location.href='/newpage' was used it made it so that the directives from the set_cache_buster were being ignored, and the browser's bfcache was showing outdated content.

So if some action causes location.href to be used, the force-cache-reload class should be added to the thing that will be clicked.

I've added a note about this to Hacking Workbench in this section

#17 Updated by Phil Hodgson almost 5 years ago

  • Status changed from In Progress to Resolved

I took the liberty of activating, for the project page's tabs, the bootstrap-tab-history gem's feature for rewriting the URL with the anchor of the currently selected tab. Works excellently for bookmarking!

Merged.

Also available in: Atom PDF