fishtest icon indicating copy to clipboard operation
fishtest copied to clipboard

"back" does not restore the button state

Open vdbergh opened this issue 2 years ago • 7 comments

Steps to trigger

  • Make sure that the default state of the "Finished" button is "Show" (i.e. the content of the corresponding div is hidden).

  • Press the "Finished" button. The state now becomes "Hide" and the content of the div becomes visible.

  • Click on one of the tests in the div.

  • Press the browser back button. The state of the "Finished" button is again "Show" (the content of the div is hidden).

EDIT:

  • If we then reload the page, the state becomes "Hide" again. So the cookie was set correctly.

vdbergh avatar Aug 14 '22 06:08 vdbergh

hello @vdbergh, may I ask what browser/device you use? this issue does not reproduce in my Chrome desktop and Chrome Android

Update_000: Nor with Firefox on windows

peregrineshahin avatar Aug 14 '22 06:08 peregrineshahin

It happens with the latest Firefox on Ubuntu 22.04. I can confirm it does not happen with other browsers I tried.

EDIT: So it looks like a FF bug to me, although I do not know what the specs say.

vdbergh avatar Aug 14 '22 07:08 vdbergh

Confirmed with Edge on Windows also, I closed a recent PR of mine due to browsers' different back button behaviors, it seems that this issue is based on different browsers saving the cache of either the last render it did or the first render when the page first loaded or not using a cache page at all (in my case) the only/best solution I see for unifying the behavior between browsers is to force/trigger a reload when window.onpageshow but this comes with the cost of first speed for those who prefer the back button to run tests and a second in triggering this event for every page in the code,

but this will have the benefit of simplifying a lot of code inside the already existing lines in when window.onpageshow in test_run to only one line of reloading the page

I'm very fond of putting

      // If browser back button was used, flush cache
      // This ensures that user will always see an
      // up-to-date view based on their cookie
      (() => {
        window.onpageshow = function (event) {
          if (event.persisted) {
            window.location.reload();
          }
        };
      })();

in every template mak,

Alternatively we can only force reload in all templates except test_run

peregrineshahin avatar Aug 14 '22 07:08 peregrineshahin

Personally I feel that buttons not remembering their state looks a bit unprofessional (even if it is not our fault). So I would be in favor of incorporating a solution to this, even if it makes the back button slower.

vdbergh avatar Aug 14 '22 08:08 vdbergh

Wait I over-thought the issue, why set the visibility twice when you can set it at once without reloading (UPDATE: failed try, because the Mako template renderer is much faster than a script tag inside it, so user will spot buttons without text inside them first)

window.onpageshow act both for cached page and a genuine load/reload no need to set visibility in the HTML also

peregrineshahin avatar Aug 14 '22 10:08 peregrineshahin

I think I once tried to do it that way but it caused flashing (if the visibility state in the HTML disagreed with the visibility in the cookie state).

EDIT:

Not sure if I used window.onpageshow though.

vdbergh avatar Aug 14 '22 10:08 vdbergh

will wait until this gets tested in DEV and see if a reload is necessary, Update: looks like it, and the PR is rebased on the reload idea

peregrineshahin avatar Aug 14 '22 11:08 peregrineshahin