From c726709d24dca412bee6e121f64bfd6b1af459fe Mon Sep 17 00:00:00 2001 From: Florian Scherf Date: Mon, 18 Mar 2024 20:49:38 +0100 Subject: [PATCH] client: fix problems with the browser history The browser history of the client never *really* worked. When you were redirected to a Lona app from an external site, you couldn't go back by clicking on the back button. When you were three links into an Lona app and refreshed the tab, the history before the refresh was unreachable. Previously, both client1 and client2 created a new history entry, using `history.pushState`, when a new view started, regardless the reason of the view starting. So initial setup, refreshes, and reconnects also created history items. This is not correct! `history.pushState` should only be called when you want to *extend* the browser history, beyond the normal history behaviour. This patch limits all calls to `history.pushState` to cases of actual in-app navigation, and adds a test to check the now corrected behaviour. Signed-off-by: Florian Scherf --- lona/client/_lona/client/input-events.js | 6 +- lona/client/_lona/client/window.js | 15 ++-- lona/client2/_lona/client2/input-events.js | 6 +- lona/client2/_lona/client2/window.js | 15 ++-- tests/test_0307_history.py | 94 ++++++++++++++++++++++ 5 files changed, 116 insertions(+), 20 deletions(-) create mode 100644 tests/test_0307_history.py diff --git a/lona/client/_lona/client/input-events.js b/lona/client/_lona/client/input-events.js index 168b1591..f0829d9d 100644 --- a/lona/client/_lona/client/input-events.js +++ b/lona/client/_lona/client/input-events.js @@ -91,7 +91,7 @@ export class LonaInputEventHandler { event.stopPropagation(); try { - lona_window.run_view(node.href); + lona_window.run_view(node.href, {}, true); } catch(error) { lona_window.crash(error); @@ -253,10 +253,10 @@ export class LonaInputEventHandler { href += '?' + query; }; - lona_window.run_view(href); + lona_window.run_view(href, {}, true); } else if(method == 'post') { - lona_window.run_view(action, form_data); + lona_window.run_view(action, form_data, true); }; diff --git a/lona/client/_lona/client/window.js b/lona/client/_lona/client/window.js index e973d3eb..8fa68397 100644 --- a/lona/client/_lona/client/window.js +++ b/lona/client/_lona/client/window.js @@ -271,10 +271,6 @@ export class LonaWindow { this._view_runtime_id = view_runtime_id; this._view_running = true; - if(this.lona_context.settings.update_address_bar) { - history.pushState({}, '', this.get_url().href); - }; - this._clear(); this._clear_node_cache(); @@ -285,7 +281,7 @@ export class LonaWindow { // TODO: implement loop detection if(this.lona_context.settings.follow_redirects) { - this.run_view(payload); + this.run_view(payload, {}, true); } else { console.debug( @@ -355,7 +351,7 @@ export class LonaWindow { }; }; - run_view(url, post_data) { + run_view(url, post_data, add_to_history) { // Save the requested url to only show HTML messages that are related // to this request. // This prevents glitches when switching urls fast. @@ -369,6 +365,11 @@ export class LonaWindow { this._view_runtime_id = undefined; this._set_url(url); + // add url to history + if(this.lona_context.settings.update_address_bar && add_to_history) { + history.pushState({}, '', this.get_url().href); + }; + // reset view start timeout if(this._view_start_timeout != undefined) { clearTimeout(this._view_start_timeout); @@ -412,6 +413,6 @@ export class LonaWindow { }; setup(url) { - this.run_view(url); + this.run_view(url, {}, false); }; }; diff --git a/lona/client2/_lona/client2/input-events.js b/lona/client2/_lona/client2/input-events.js index ce7f072e..afe0038f 100644 --- a/lona/client2/_lona/client2/input-events.js +++ b/lona/client2/_lona/client2/input-events.js @@ -91,7 +91,7 @@ export class LonaInputEventHandler { event.stopPropagation(); try { - lona_window.run_view(node.href); + lona_window.run_view(node.href, {}, true); } catch(error) { lona_window.crash(error); @@ -253,10 +253,10 @@ export class LonaInputEventHandler { href += '?' + query; }; - lona_window.run_view(href); + lona_window.run_view(href, {}, true); } else if(method == 'post') { - lona_window.run_view(action, form_data); + lona_window.run_view(action, form_data, true); }; diff --git a/lona/client2/_lona/client2/window.js b/lona/client2/_lona/client2/window.js index 27df9137..727187f7 100644 --- a/lona/client2/_lona/client2/window.js +++ b/lona/client2/_lona/client2/window.js @@ -104,10 +104,6 @@ export class LonaWindow { this._view_runtime_id = view_runtime_id; this._view_running = true; - if(this.lona_context.settings.update_address_bar) { - history.pushState({}, '', this.get_url().href); - }; - this._rendering_engine._clear(); this._rendering_engine._clear_node_cache(); @@ -118,7 +114,7 @@ export class LonaWindow { // TODO: implement loop detection if(this.lona_context.settings.follow_redirects) { - this.run_view(payload); + this.run_view(payload, {}, true); } else { console.debug( @@ -188,7 +184,7 @@ export class LonaWindow { }; }; - run_view(url, post_data) { + run_view(url, post_data, add_to_history) { // Save the requested url to only show HTML messages that are related // to this request. // This prevents glitches when switching urls fast. @@ -202,6 +198,11 @@ export class LonaWindow { this._view_runtime_id = undefined; this._set_url(url); + // add url to history + if(this.lona_context.settings.update_address_bar && add_to_history) { + history.pushState({}, '', this.get_url().href); + }; + // reset view start timeout if(this._view_start_timeout != undefined) { clearTimeout(this._view_start_timeout); @@ -245,6 +246,6 @@ export class LonaWindow { }; setup(url) { - this.run_view(url); + this.run_view(url, {}, false); }; }; diff --git a/tests/test_0307_history.py b/tests/test_0307_history.py new file mode 100644 index 00000000..b9f27327 --- /dev/null +++ b/tests/test_0307_history.py @@ -0,0 +1,94 @@ +import pytest + + +@pytest.mark.parametrize('client_version', [1, 2]) +@pytest.mark.parametrize('browser_name', ['chromium', 'firefox', 'webkit']) +async def test_client_history( + browser_name, + client_version, + lona_app_context, +): + + """ + This test checks if the browser history works correctly by navigating + a browser page using the back and forward "buttons". + """ + + from playwright.async_api import async_playwright + + from lona.html import HTML, H1, A + from lona import View + + def setup_app(app): + + @app.route('/') + class TestView(View): + def handle_request(self, request): + return HTML( + H1(request.match_info['title']), + A('foo', href='/foo', _id='foo'), + A('bar', href='/bar', _id='bar'), + A('baz', href='/baz', _id='baz'), + ) + + @app.route('/') + class Index(View): + def handle_request(self, request): + return HTML( + H1('index'), + A('foo', href='/foo', _id='foo'), + A('bar', href='/bar', _id='bar'), + A('baz', href='/baz', _id='baz'), + ) + + context = await lona_app_context(setup_app) + + async with async_playwright() as p: + browser = await p.chromium.launch() + browser_context = await browser.new_context() + page = await browser_context.new_page() + + # external site, about:blank + await page.goto('about:blank') + await page.wait_for_url('about:blank') + + # / + await page.goto(context.make_url('/')) + await page.wait_for_url('/') + await page.wait_for_selector('h1:has-text("index")') + + # /foo + await page.click('#foo') + await page.wait_for_url('/foo') + await page.wait_for_selector('h1:has-text("foo")') + + # /bar + await page.click('#bar') + await page.wait_for_url('/bar') + await page.wait_for_selector('h1:has-text("bar")') + + # back to /foo + await page.go_back() + await page.wait_for_url('/foo') + await page.wait_for_selector('h1:has-text("foo")') + + # forward to /bar + await page.go_forward() + await page.wait_for_url('/bar') + await page.wait_for_selector('h1:has-text("bar")') + + # back to / + await page.go_back() + await page.go_back() + await page.wait_for_url('/') + await page.wait_for_selector('h1:has-text("index")') + + # back to external site + await page.go_back() + await page.wait_for_url('about:blank') + + # forward to /foo + await page.go_forward() + await page.go_forward() + await page.wait_for_url('/foo') + await page.wait_for_selector('h1:has-text("foo")')