From 3ec91874c071f38cf788099abcbf930a28c1189a Mon Sep 17 00:00:00 2001 From: Florian Scherf Date: Fri, 17 Mar 2023 18:35:19 +0100 Subject: [PATCH 01/18] html: NodeList: reset: remove duplicate patches `NodeList.reset()` gets called with a list of nodes when the `Node.nodes` property gets set. `NodeList.reset()` then clears its node list and creates a `lona.protocol.OPERATION.RESET` patch. Previously, the code falsely create a patch for every new node, that contained all following nodes. That resulted in a list of patches that would override each other on the client. This was no problem in the past, because the client has no checks if a node was already sent by the server, and because the patches overwrote each other, the HTML end-result always was correct. This patch changes the code to create only one reset patch, instead of multiple ones. Signed-off-by: Florian Scherf --- lona/html/node_list.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lona/html/node_list.py b/lona/html/node_list.py index 26b5753f..ba45df89 100644 --- a/lona/html/node_list.py +++ b/lona/html/node_list.py @@ -213,14 +213,14 @@ def _reset(self, values): self._nodes.append(node) - self._node.document.add_patch( - node_id=self._node.id, - patch_type=PATCH_TYPE.NODES, - operation=OPERATION.RESET, - payload=[ - [i._serialize() for i in self._nodes], - ], - ) + self._node.document.add_patch( + node_id=self._node.id, + patch_type=PATCH_TYPE.NODES, + operation=OPERATION.RESET, + payload=[ + [i._serialize() for i in self._nodes], + ], + ) def _serialize(self, include_node_ids=True): return [i._serialize(include_node_ids=include_node_ids) From e03ae526973f22c64578728c8a971ce326861a05 Mon Sep 17 00:00:00 2001 From: Florian Scherf Date: Sat, 18 Mar 2023 15:13:27 +0100 Subject: [PATCH 02/18] tests: html: use lona.html.HTML1 explicitly This patch changes all tests that use `lona.html.HTML` to use `lona.html.HTML1` explicitly, to make room for tests for `lona.html.HTML2`. Signed-off-by: Florian Scherf --- tests/test_0001_html.py | 94 ++++++++++++++++++++--------------------- 1 file changed, 47 insertions(+), 47 deletions(-) diff --git a/tests/test_0001_html.py b/tests/test_0001_html.py index e1c4538a..2958ee36 100644 --- a/tests/test_0001_html.py +++ b/tests/test_0001_html.py @@ -13,9 +13,9 @@ Select, Option, Button, + HTML1, Span, Node, - HTML, Div, H1, ) @@ -389,9 +389,9 @@ def test_list(self): @pytest.mark.incremental() -class TestHTMLFromStr: +class TestLegacyHtmlParsing: def test_empty_node(self): - node = HTML('
')[0] + node = HTML1('
')[0] assert node.tag_name == 'div' assert node.id_list == [] @@ -401,7 +401,7 @@ def test_empty_node(self): assert node.nodes == [] def test_node_with_attributes(self): - node = HTML(""" + node = HTML1("""
""")[0] @@ -414,7 +414,7 @@ def test_node_with_attributes(self): assert node.nodes == [] def test_sub_nodes(self): - node = HTML(""" + node = HTML1("""
@@ -429,17 +429,17 @@ def test_sub_nodes(self): assert node.nodes[2].tag_name == 'h1' def test_multiple_ids(self): - node = HTML('
')[0] + node = HTML1('
')[0] assert node.id_list == ['foo', 'bar', 'baz'] def test_multiple_classes(self): - node = HTML('
')[0] + node = HTML1('
')[0] assert node.class_list == ['foo', 'bar', 'baz'] def test_multiple_styles(self): - node = HTML('
')[0] + node = HTML1('
')[0] assert node.style == { 'color': 'black', @@ -447,7 +447,7 @@ def test_multiple_styles(self): } def test_multiple_attributes(self): - node = HTML('
')[0] + node = HTML1('
')[0] assert node.attributes == { 'foo': 'bar', @@ -455,17 +455,17 @@ def test_multiple_attributes(self): } def test_high_level_nodes(self): - node = HTML('')[0] + node = HTML1('')[0] assert type(node) is Button def test_boolean_property_without_value(self): - node = HTML('')[0] + node = HTML1('')[0] assert node.disabled def test_boolean_property_with_value(self): - node = HTML('')[0] + node = HTML1('')[0] assert node.disabled @@ -474,69 +474,69 @@ def test_missing_end_tag(self): ValueError, match='Invalid html: missing end tag ', ): - HTML('') + HTML1('') def test_wrong_end_tag(self): with pytest.raises( ValueError, match='Invalid html:
expected, received', ): - HTML('

abc

') + HTML1('

abc

') def test_end_tag_without_start_tag(self): with pytest.raises( ValueError, match='Invalid html: missing start tag for ', ): - HTML('
abc
') + HTML1('
abc
') def test_missing_start_tag(self): with pytest.raises( ValueError, match='Invalid html: missing start tag for ', ): - HTML('') + HTML1('') def test_self_closing_tag_with_slash(self): - img = HTML('
')[0].nodes[0] + img = HTML1('
')[0].nodes[0] assert img.tag_name == 'img' assert img.self_closing_tag is True def test_self_closing_tag_without_slash(self): - img = HTML('
')[0].nodes[0] + img = HTML1('
')[0].nodes[0] assert img.tag_name == 'img' assert img.self_closing_tag is True def test_non_self_closing_tag(self): - div = HTML('
')[0] + div = HTML1('
')[0] assert div.tag_name == 'div' assert div.self_closing_tag is False def test_non_self_closing_tag_with_slash(self): - span = HTML('
')[0].nodes[0] + span = HTML1('
')[0].nodes[0] assert span.tag_name == 'span' assert span.self_closing_tag is True def test_default_input_type_is_text(self): - node = HTML('')[0] + node = HTML1('')[0] assert type(node) is TextInput assert node.value == 'abc' assert node.disabled is False def test_input_type_text(self): - node = HTML('')[0] + node = HTML1('')[0] assert type(node) is TextInput assert node.value == 'xyz' assert node.disabled is True def test_input_type_unknown(self): - node = HTML('')[0] + node = HTML1('')[0] assert type(node) is Node @@ -564,51 +564,51 @@ def test_input_type_unknown(self): ], ) def test_not_implemented_input_types(self, tp): - node = HTML(f'')[0] + node = HTML1(f'')[0] assert type(node) is Node def test_input_type_number(self): - node = HTML('')[0] + node = HTML1('')[0] assert type(node) is NumberInput assert node.value == 123.5 def test_input_type_checkbox(self): - node = HTML('')[0] + node = HTML1('')[0] assert type(node) is CheckBox assert node.value is False def test_input_type_checkbox_checked(self): - node = HTML('')[0] + node = HTML1('')[0] assert type(node) is CheckBox assert node.value is True def test_input_type_submit(self): - node = HTML('')[0] + node = HTML1('')[0] assert type(node) is Submit def test_textarea(self): - node = HTML('')[0] + node = HTML1('')[0] assert type(node) is TextArea assert node.value == 'abc' def test_textarea_with_self_closing_tag_inside(self): - textarea = HTML('')[0] + textarea = HTML1('')[0] assert textarea.value == 'abc
xyz' def test_textarea_with_pair_tag_inside(self): - textarea = HTML('')[0] + textarea = HTML1('')[0] assert textarea.value == 'aaa bbb ccc' def test_select(self): - node = HTML(""" + node = HTML1(""" @@ -660,7 +660,7 @@ def test_change_value(self): assert node.value == 12.3 def test_parsing_no_attributes(self): - node = HTML('')[0] + node = HTML1('')[0] assert node.value is None assert node.min is None @@ -668,41 +668,41 @@ def test_parsing_no_attributes(self): assert node.step is None def test_parsing_int_value(self): - node = HTML('')[0] + node = HTML1('')[0] assert node.value == 123 def test_parsing_float_value(self): - node = HTML('')[0] + node = HTML1('')[0] assert node.value == 12.3 def test_parsing_broken_step(self): - node = HTML('')[0] + node = HTML1('')[0] assert node.value == 123 assert node.step is None def test_parsing_int_step(self): - node = HTML('')[0] + node = HTML1('')[0] assert node.value == 12.3 assert node.step == 3 def test_parsing_float_step(self): - node = HTML('')[0] + node = HTML1('')[0] assert node.value == 12.3 assert node.step == 0.1 def test_parsing_broken_value(self): - node = HTML('')[0] + node = HTML1('')[0] assert node.raw_value == 'abc' assert node.value is None def test_parsing_all_attributes(self): - node = HTML( + node = HTML1( '', )[0] @@ -717,19 +717,19 @@ def test_attribute_escaping(self): assert node.style['font-family'] == '"Times New Roman"' assert node.style.to_sub_attribute_string() == 'font-family: "Times New Roman"' # NOQA: E501 - node = HTML(str(node))[0] + node = HTML1(str(node))[0] assert node.style['font-family'] == '"Times New Roman"' # selectors ############################################################### def test_unsupported_selector(self): with pytest.raises(ValueError, match='unsupported selector feature:*'): - HTML().query_selector('div > div') + HTML1().query_selector('div > div') def test_query_selector(self): # test selectors by tag name html = Div( - HTML(), + HTML1(), 'foo', Div( H1(), @@ -743,7 +743,7 @@ def test_query_selector(self): # test selectors by id html = Div( - HTML(), + HTML1(), 'foo', Div( Div(_id='foo'), @@ -757,7 +757,7 @@ def test_query_selector(self): # test selectors by class html = Div( - HTML(), + HTML1(), 'foo', Div( Div(_class='foo'), @@ -771,7 +771,7 @@ def test_query_selector(self): # test selectors by attribute html = Div( - HTML(), + HTML1(), 'foo', Div( Div(_type='foo'), From f95fed5e65f10387c7272a83f647ac29ded38a32 Mon Sep 17 00:00:00 2001 From: Florian Scherf Date: Thu, 16 Mar 2023 21:27:28 +0100 Subject: [PATCH 03/18] html: HTML2: fix string parsing Previously, `lona.parsing.HTML2` used `lona.parsing.HTML2` to parse given HTML-strings. Since `lona.parsing.HTML2` wraps all nodes on the top-level of the parsing result, `lona.parsing.HTML2` calls, that contained nested HTML-strings, resulted in node trees with falsely wrapped sub node trees. This patch updates `lona.parsing.HTML2` to use `lona.parsing.html_string_to_node_list` for given HTML-strings instead, so the final wrapping only happens on the top-level, and not down the tree. Signed-off-by: Florian Scherf --- lona/html/parsing.py | 14 +++++++------ tests/test_0001_html.py | 46 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/lona/html/parsing.py b/lona/html/parsing.py index 471af9d9..66b59255 100644 --- a/lona/html/parsing.py +++ b/lona/html/parsing.py @@ -246,15 +246,17 @@ def HTML( # html string elif '<' in node or '>' in node: + parsed_nodes = html_string_to_node_list( + html_string=node, + use_high_level_nodes=use_high_level_nodes, + node_classes=node_classes or {}, + ) + if len(nodes) > 1: - _nodes.append(HTML(node)) + _nodes.extend(parsed_nodes) else: - _nodes = html_string_to_node_list( - html_string=node, - use_high_level_nodes=use_high_level_nodes, - node_classes=node_classes or {}, - ) + _nodes = parsed_nodes else: _nodes.append(TextNode(node)) diff --git a/tests/test_0001_html.py b/tests/test_0001_html.py index 2958ee36..41c0ce20 100644 --- a/tests/test_0001_html.py +++ b/tests/test_0001_html.py @@ -13,6 +13,7 @@ Select, Option, Button, + HTML2, HTML1, Span, Node, @@ -638,6 +639,51 @@ def test_select2(self): set_use_future_node_classes(False) +@pytest.mark.incremental() +class TestHtmlParsing: + def test_sub_nodes(self): + node = HTML2(""" +
+ +
+

+
+ """) + + assert node.tag_name == 'div' + assert len(node.nodes) == 3 + assert node.nodes[0].tag_name == 'span' + assert node.nodes[1].tag_name == 'div' + assert node.nodes[2].tag_name == 'h1' + + def test_wrapping(self): + node = HTML2(""" + +
+

+ """) + + assert node.tag_name == 'div' + assert len(node.nodes) == 3 + assert node.nodes[0].tag_name == 'span' + assert node.nodes[1].tag_name == 'div' + assert node.nodes[2].tag_name == 'h1' + + def test_multiple_strings(self): + node = HTML2( + '', + '
', + '

', + ) + + assert node.tag_name == 'div' + assert len(node.nodes) == 4 + assert node.nodes[0].tag_name == 'span' + assert node.nodes[1].tag_name == 'span' + assert node.nodes[2].tag_name == 'div' + assert node.nodes[3].tag_name == 'h1' + + @pytest.mark.incremental() class TestNumberInput: def test_default_properties(self): From 138de6fa78856294fd13626f9f5dee9a19f8d8ff Mon Sep 17 00:00:00 2001 From: Florian Scherf Date: Tue, 14 Mar 2023 19:53:46 +0100 Subject: [PATCH 04/18] test project: use client2 by default Since client2 will be the client implementation of Lona 2, and Lona 2 is just around the corner, all testing and debugging should be done against client2 first. Signed-off-by: Florian Scherf --- test_project/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_project/Makefile b/test_project/Makefile index 4af1a602..504229ad 100644 --- a/test_project/Makefile +++ b/test_project/Makefile @@ -3,7 +3,7 @@ PYTHON_VENV=env LONA_SHELL_SERVER_URL=file://socket LONA_DEFAULT_ARGS=--shell-server-url=$(LONA_SHELL_SERVER_URL) -CLIENT_VERSION=1 +CLIENT_VERSION=2 all: server From a31fc18c6a512898426bf0528f98dbc97a33afca Mon Sep 17 00:00:00 2001 From: Florian Scherf Date: Tue, 14 Mar 2023 22:32:48 +0100 Subject: [PATCH 05/18] client2: add support for HTML symbols This patch adds support for HTML symbols: https://www.w3schools.com/html/html_symbols.asp Signed-off-by: Florian Scherf --- lona/client2/_lona/client2/rendering-engine.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lona/client2/_lona/client2/rendering-engine.js b/lona/client2/_lona/client2/rendering-engine.js index e922a6c1..d89a73d9 100644 --- a/lona/client2/_lona/client2/rendering-engine.js +++ b/lona/client2/_lona/client2/rendering-engine.js @@ -42,6 +42,7 @@ export class LonaRenderingEngine { this._widgets = new Map(); this._widgets_to_setup = new Array(); this._widgets_to_update = new Array(); + this._dom_parser = new DOMParser(); }; // helper ----------------------------------------------------------------- @@ -101,6 +102,13 @@ export class LonaRenderingEngine { node.innerHTML = ''; }; + _parse_html_string(html_string) { + return this._dom_parser.parseFromString( + html_string, + 'text/html', + ).documentElement.textContent; + } + // node cache ------------------------------------------------------------- _clear_node_cache() { // running widget deconstructors @@ -167,7 +175,7 @@ export class LonaRenderingEngine { // TextNode if(node_type == Lona.protocol.NODE_TYPE.TEXT_NODE) { const node_id = node_spec[1]; - const node_content = node_spec[2]; + const node_content = this._parse_html_string(node_spec[2]); const node = document.createTextNode(node_content); From 6d9179ff8b55b240f2b3bf31cee1bdbfdcdc1939 Mon Sep 17 00:00:00 2001 From: Florian Scherf Date: Tue, 14 Mar 2023 23:05:17 +0100 Subject: [PATCH 06/18] client: add support for HTML symbols This patch adds support for HTML symbols: https://www.w3schools.com/html/html_symbols.asp Signed-off-by: Florian Scherf --- lona/client/_lona/client/dom-renderer.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lona/client/_lona/client/dom-renderer.js b/lona/client/_lona/client/dom-renderer.js index e50720a3..e8cadf3d 100644 --- a/lona/client/_lona/client/dom-renderer.js +++ b/lona/client/_lona/client/dom-renderer.js @@ -30,8 +30,17 @@ export class LonaDomRenderer { constructor(lona_context, lona_window) { this.lona_context = lona_context; this.lona_window = lona_window; + + this._dom_parser = new DOMParser(); }; + _parse_html_string(html_string) { + return this._dom_parser.parseFromString( + html_string, + 'text/html', + ).documentElement.textContent; + } + // html rendering --------------------------------------------------------- _render_node(node_spec) { var property_names = ['value', 'checked', 'selected']; @@ -132,7 +141,7 @@ export class LonaDomRenderer { // TextNode } else if(node_type == Lona.protocol.NODE_TYPE.TEXT_NODE) { var node_id = node_spec[1]; - var node_content = node_spec[2]; + var node_content = this._parse_html_string(node_spec[2]); var node = document.createTextNode(node_content); From d2a9d4239493f61e5cc75a62dea9c4e4d446068b Mon Sep 17 00:00:00 2001 From: Florian Scherf Date: Tue, 14 Mar 2023 22:52:56 +0100 Subject: [PATCH 07/18] tests: rendering: add test for HTML symbols This patch adds a test to ensure HTML symbols get rendered correctly on the client. https://www.w3schools.com/html/html_symbols.asp Signed-off-by: Florian Scherf --- test_project/views/frontend/rendering.py | 110 +++++++++++++---------- tests/test_rendering.py | 34 ++++--- 2 files changed, 86 insertions(+), 58 deletions(-) diff --git a/test_project/views/frontend/rendering.py b/test_project/views/frontend/rendering.py index 25c6a458..06f755eb 100644 --- a/test_project/views/frontend/rendering.py +++ b/test_project/views/frontend/rendering.py @@ -452,45 +452,63 @@ def step_21(self): self.rendering_root.nodes[0].attributes.clear() - # style tests @client_version(1, 2) def step_22(self): - self.set_step_label(22, 'Set style') + self.set_step_label(22, 'HTML Symbols') self.rendering_root.nodes = [ - Div(_style='top: 1px; right: 2px;'), + '€', + '€', + '€', ] + # style tests @client_version(1, 2) def step_23(self): - self.set_step_label(23, 'Add style') + self.set_step_label(23, 'Empty style') - self.rendering_root.nodes[0].style['bottom'] = '3px' + self.rendering_root.nodes = [ + Div(), + ] @client_version(1, 2) def step_24(self): - self.set_step_label(24, 'Remove style') + self.set_step_label(24, 'Set style') - del self.rendering_root.nodes[0].style['top'] + self.rendering_root.nodes = [ + Div(_style='top: 1px; right: 2px;'), + ] @client_version(1, 2) def step_25(self): - self.set_step_label(25, 'Reset style') + self.set_step_label(25, 'Add style') + + self.rendering_root.nodes[0].style['bottom'] = '3px' + + @client_version(1, 2) + def step_26(self): + self.set_step_label(26, 'Remove style') + + del self.rendering_root.nodes[0].style['top'] + + @client_version(1, 2) + def step_27(self): + self.set_step_label(27, 'Reset style') self.rendering_root.nodes[0].style = { 'left': '4px', } @client_version(1, 2) - def step_26(self): - self.set_step_label(26, 'Clear style') + def step_28(self): + self.set_step_label(28, 'Clear style') self.rendering_root.nodes[0].style.clear() # widget data tests @client_version(1, 2) - def step_27(self): - self.set_step_label(27, 'Widget Data: list: setup') + def step_29(self): + self.set_step_label(29, 'Widget Data: list: setup') self.rendering_root.nodes = [ WidgetDataTestComponent( @@ -499,8 +517,8 @@ def step_27(self): ] @client_version(1, 2) - def step_28(self): - self.set_step_label(28, 'Widget Data: list: append') + def step_30(self): + self.set_step_label(30, 'Widget Data: list: append') component = self.rendering_root.nodes[0] @@ -510,8 +528,8 @@ def step_28(self): component.update_state() @client_version(1, 2) - def step_29(self): - self.set_step_label(29, 'Widget Data: list: remove') + def step_31(self): + self.set_step_label(31, 'Widget Data: list: remove') component = self.rendering_root.nodes[0] @@ -519,8 +537,8 @@ def step_29(self): component.update_state() @client_version(1, 2) - def step_30(self): - self.set_step_label(30, 'Widget Data: list: insert') + def step_32(self): + self.set_step_label(32, 'Widget Data: list: insert') component = self.rendering_root.nodes[0] @@ -528,8 +546,8 @@ def step_30(self): component.update_state() @client_version(1, 2) - def step_31(self): - self.set_step_label(31, 'Widget Data: list: clear') + def step_33(self): + self.set_step_label(33, 'Widget Data: list: clear') component = self.rendering_root.nodes[0] @@ -537,8 +555,8 @@ def step_31(self): component.update_state() @client_version(1, 2) - def step_32(self): - self.set_step_label(32, 'Widget Data: list: reset') + def step_34(self): + self.set_step_label(34, 'Widget Data: list: reset') component = self.rendering_root.nodes[0] @@ -546,8 +564,8 @@ def step_32(self): component.update_state() @client_version(1, 2) - def step_33(self): - self.set_step_label(33, 'Widget Data: dict: setup') + def step_35(self): + self.set_step_label(35, 'Widget Data: dict: setup') component = self.rendering_root.nodes[0] @@ -555,8 +573,8 @@ def step_33(self): component.update_state() @client_version(1, 2) - def step_34(self): - self.set_step_label(34, 'Widget Data: dict: set') + def step_36(self): + self.set_step_label(36, 'Widget Data: dict: set') component = self.rendering_root.nodes[0] @@ -566,8 +584,8 @@ def step_34(self): component.update_state() @client_version(1, 2) - def step_35(self): - self.set_step_label(35, 'Widget Data: dict: del') + def step_37(self): + self.set_step_label(37, 'Widget Data: dict: del') component = self.rendering_root.nodes[0] @@ -575,8 +593,8 @@ def step_35(self): component.update_state() @client_version(1, 2) - def step_36(self): - self.set_step_label(36, 'Widget Data: dict: pop') + def step_38(self): + self.set_step_label(38, 'Widget Data: dict: pop') component = self.rendering_root.nodes[0] @@ -584,8 +602,8 @@ def step_36(self): component.update_state() @client_version(1, 2) - def step_37(self): - self.set_step_label(37, 'Widget Data: dict: clear') + def step_39(self): + self.set_step_label(39, 'Widget Data: dict: clear') component = self.rendering_root.nodes[0] @@ -593,8 +611,8 @@ def step_37(self): component.update_state() @client_version(1, 2) - def step_38(self): - self.set_step_label(38, 'Widget Data: dict: reset') + def step_40(self): + self.set_step_label(40, 'Widget Data: dict: reset') component = self.rendering_root.nodes[0] @@ -605,8 +623,8 @@ def step_38(self): # TODO: remove in 2.0 @client_version(1) - def step_39(self): - self.set_step_label(39, 'Legacy Widgets: Setup') + def step_41(self): + self.set_step_label(41, 'Legacy Widgets: Setup') self.rendering_root.clear() @@ -623,8 +641,8 @@ def step_39(self): ] @client_version(1) - def step_40(self): - self.set_step_label(40, 'Legacy Widgets: Append Nodes') + def step_42(self): + self.set_step_label(42, 'Legacy Widgets: Append Nodes') widget1 = self.rendering_root.nodes[0] widget1.append(Div('1.3')) @@ -635,8 +653,8 @@ def step_40(self): self.rendering_root.append(Div('4.1')) @client_version(1) - def step_41(self): - self.set_step_label(41, 'Legacy Widgets: Set Nodes') + def step_43(self): + self.set_step_label(43, 'Legacy Widgets: Set Nodes') widget1 = self.rendering_root.nodes[0] widget1.nodes[1] = Div('1.2.1') @@ -645,8 +663,8 @@ def step_41(self): widget1.nodes[1] = Div('3.2.1') @client_version(1) - def step_42(self): - self.set_step_label(42, 'Legacy Widgets: Reset Nodes') + def step_44(self): + self.set_step_label(44, 'Legacy Widgets: Reset Nodes') widget1 = self.rendering_root.nodes[0] @@ -669,8 +687,8 @@ def step_42(self): self.rendering_root[3] = Div('4.1.1') @client_version(1) - def step_43(self): - self.set_step_label(43, 'Legacy Widgets: Insert Nodes') + def step_45(self): + self.set_step_label(45, 'Legacy Widgets: Insert Nodes') widget1 = self.rendering_root[0] widget1.nodes.insert(2, Div('1.2.1.1')) @@ -681,8 +699,8 @@ def step_43(self): widget2.nodes.insert(2, Div('3.2.1.1')) @client_version(1) - def step_44(self): - self.set_step_label(44, 'Legacy Widgets: Remove Nodes') + def step_46(self): + self.set_step_label(46, 'Legacy Widgets: Remove Nodes') widget1 = self.rendering_root[0] widget1.nodes.pop(2) diff --git a/tests/test_rendering.py b/tests/test_rendering.py index 4f479165..518a79fc 100644 --- a/tests/test_rendering.py +++ b/tests/test_rendering.py @@ -123,11 +123,21 @@ async def parse_json(page, locator): assert html.nodes == context.server.state['rendering-root'].nodes + # html symbols + await next_step(page, 22) + + html_string = await rendering_root_element.inner_html() + + assert html_string == '€€€' + # CSS tests ########################################################### + + # 23 Empty style + await next_step(page, 23) await check_default_styles(page) - # 22 Set Style - await next_step(page, 22) + # 24 Set Style + await next_step(page, 24) computed_style = await get_computed_style(page) @@ -136,8 +146,8 @@ async def parse_json(page, locator): assert computed_style['bottom'] == 'auto' assert computed_style['left'] == 'auto' - # 23 Add Style - await next_step(page, 23) + # 25 Add Style + await next_step(page, 25) computed_style = await get_computed_style(page) style = get_style() @@ -147,8 +157,8 @@ async def parse_json(page, locator): assert computed_style['bottom'] == '3px' assert computed_style['left'] == 'auto' - # 24 Remove Style - await next_step(page, 24) + # 26 Remove Style + await next_step(page, 26) computed_style = await get_computed_style(page) style = get_style() @@ -160,8 +170,8 @@ async def parse_json(page, locator): assert computed_style['bottom'] == '3px' assert computed_style['left'] == 'auto' - # 25 Reset Style - await next_step(page, 25) + # 27 Reset Style + await next_step(page, 27) computed_style = await get_computed_style(page) style = get_style() @@ -174,12 +184,12 @@ async def parse_json(page, locator): assert computed_style['bottom'] == 'auto' assert computed_style['left'] == '4px' - # 26 Clear Style - await next_step(page, 26) + # 28 Clear Style + await next_step(page, 28) await check_default_styles(page) # widget data tests ################################################### - for step in range(27, 39): + for step in range(29, 41): await next_step(page, step) server_widget_data = await parse_json( @@ -200,7 +210,7 @@ async def parse_json(page, locator): if get_client_version() != 1: return - for step in range(39, 45): + for step in range(41, 47): await next_step(page, step) client_html_string = await rendering_root_element.inner_html() From 18ef959ff297ab9f33a1cd7d890fa50bd4648ead Mon Sep 17 00:00:00 2001 From: Florian Scherf Date: Fri, 17 Mar 2023 18:35:39 +0100 Subject: [PATCH 08/18] client2: rendering engine: add _cache_node method Previously, the rendering engine wrote the node cache directly, to add a newly rendered node, without checking if a already cached node gets overridden by it. This is dangerous and covers up potential bugs in the rendering code on the client and in the patch creation code on the server. This patch adds `_cache_node()` to the rendering engine, which checks if a node, with the given node id, is already cached, and throws an exception if so. Signed-off-by: Florian Scherf --- lona/client2/_lona/client2/rendering-engine.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lona/client2/_lona/client2/rendering-engine.js b/lona/client2/_lona/client2/rendering-engine.js index d89a73d9..3264fcc3 100644 --- a/lona/client2/_lona/client2/rendering-engine.js +++ b/lona/client2/_lona/client2/rendering-engine.js @@ -73,6 +73,14 @@ export class LonaRenderingEngine { return this._nodes.get(node_id); }; + _cache_node(node_id, node) { + if(this._nodes.has(node_id)) { + throw(`node with id ${node_id} is already cached`); + } + + this._nodes.set(node_id, node); + }; + _insert_node(node, node_id, index) { const target_node = this._get_node(node_id); @@ -179,7 +187,7 @@ export class LonaRenderingEngine { const node = document.createTextNode(node_content); - this._nodes.set(node_id, node); + this._cache_node(node_id, node); return node; }; @@ -245,7 +253,7 @@ export class LonaRenderingEngine { node.appendChild(child_node); }); - this._nodes.set(node_id, node); + this._cache_node(node_id, node); // widget if(widget_class_name != '') { From b8c82ba773e771fba8f7c1254b46a93fb4be4ad7 Mon Sep 17 00:00:00 2001 From: Florian Scherf Date: Fri, 10 Mar 2023 17:57:31 +0100 Subject: [PATCH 09/18] client2: add new widget API Previously, the Lona client- and widget-API were very Pythonic, and used only basic JavaScript class features before. In Lona 2, methods like `Widget.data_updated` will be renamed to `Widget.onDataUpdated`. To facilitate the transition, the new names and API will be added to Lona 1 and coexist with the old API, to ensure compatibility until Lona 2. This is done by adding a new JavaScript class, that abstracts the new and the old widget API. This patch adds `Widget.onDataUpdated` as a replacement for `Widget.data_updated`, `Widget.destroy` as a replacement for `Widget.deconstruct`, and adds all arguments of `Widget.setup` to `Widget.constructor`, to make a separate setup stage, besides the JavaScript constructor, obsolete in Lona 2. This patch also adds a wrapper class for widget data, to make helper methods like `WidgetData.set` and `WidgetData.get` possible in the future. Signed-off-by: Florian Scherf --- .../client2/_lona/client2/rendering-engine.js | 68 +++------ lona/client2/_lona/client2/widget.js | 137 ++++++++++++++++++ 2 files changed, 160 insertions(+), 45 deletions(-) create mode 100644 lona/client2/_lona/client2/widget.js diff --git a/lona/client2/_lona/client2/rendering-engine.js b/lona/client2/_lona/client2/rendering-engine.js index 3264fcc3..b6081403 100644 --- a/lona/client2/_lona/client2/rendering-engine.js +++ b/lona/client2/_lona/client2/rendering-engine.js @@ -24,7 +24,7 @@ SOFTWARE. 'use strict'; -import { LonaWindowShim } from './window-shim.js'; +import { Widget } from './widget.js'; import { Lona } from './lona.js'; const SPECIAL_ATTRIBUTE_NAMES = ['id', 'class', 'style', 'data-lona-node-id']; @@ -38,7 +38,6 @@ export class LonaRenderingEngine { this._root = root; this._nodes = new Map(); - this._widget_data = new Map(); this._widgets = new Map(); this._widgets_to_setup = new Array(); this._widgets_to_update = new Array(); @@ -119,16 +118,13 @@ export class LonaRenderingEngine { // node cache ------------------------------------------------------------- _clear_node_cache() { - // running widget deconstructors + // destroy widgets this._widgets.forEach(widget => { - if(widget.deconstruct !== undefined) { - widget.deconstruct(); - }; + widget.destroy_widget(); }); // resetting node state this._nodes.clear(); - this._widget_data.clear(); this._widgets.clear(); this._widgets_to_setup.length = 0; this._widgets_to_update.length = 0; @@ -151,17 +147,12 @@ export class LonaRenderingEngine { const widget = this._widgets.get(node_id); - // run deconstructor - if(widget.deconstruct !== undefined) { - widget.deconstruct(); - }; + // destroy widget + widget.destroy_widget(); // remove widget this._widgets.delete(node_id); - // remove widget data - this._widget_data.delete(node_id); - // remove widget from _widgets_to_setup if(this._widgets_to_setup.indexOf(node_id) > -1) { this._widgets_to_setup.splice( @@ -261,18 +252,16 @@ export class LonaRenderingEngine { throw(`RuntimeError: unknown widget name '${widget_class_name}'`); } - const widget_class = Lona.widget_classes[widget_class_name]; - - const window_shim = new LonaWindowShim( + const widget = new Widget( this.lona_context, this.lona_window, + node, node_id, + Lona.widget_classes[widget_class_name], + widget_data, ); - const widget = new widget_class(window_shim); - this._widgets.set(node_id, widget); - this._widget_data.set(node_id, widget_data); this._widgets_to_setup.splice(0, 0, node_id); } @@ -286,37 +275,24 @@ export class LonaRenderingEngine { _run_widget_hooks() { // setup this._widgets_to_setup.forEach(node_id => { - const widget = this._widgets.get(node_id); - const widget_data = this._widget_data.get(node_id); - - widget.data = JSON.parse(JSON.stringify(widget_data)); - - if(widget === undefined) { + if(!this._widgets.has(node_id)) { return; - }; + } - widget.nodes = [this._get_node(node_id)]; - widget.root_node = widget.nodes[0]; + const widget = this._widgets.get(node_id); - if(widget.setup !== undefined) { - widget.setup(); - }; + widget.initialize_widget(); }); // data_updated this._widgets_to_update.forEach(node_id => { - const widget = this._widgets.get(node_id); - const widget_data = this._widget_data.get(node_id); - - widget.data = JSON.parse(JSON.stringify(widget_data)); - - if(widget === undefined) { + if(!this._widgets.has(node_id)) { return; - }; + } - if(widget.data_updated !== undefined) { - widget.data_updated(); - }; + const widget = this._widgets.get(node_id); + + widget.run_data_updated_hook(); }); this._widgets_to_setup = []; @@ -542,9 +518,11 @@ export class LonaRenderingEngine { const key_path = payload[0]; const data = payload.splice(1); + const widget = this._widgets.get(node_id); + // key path let parent_data = undefined; - let widget_data = this._widget_data.get(node_id); + let widget_data = widget.raw_widget_data; let new_data = undefined; key_path.forEach(key => { @@ -559,7 +537,7 @@ export class LonaRenderingEngine { // RESET } else if(operation == Lona.protocol.OPERATION.RESET) { if(parent_data === undefined) { - this._widget_data.set(node_id, data[0]); + widget.raw_widget_data = data[0]; } else { parent_data = data[0]; @@ -577,7 +555,7 @@ export class LonaRenderingEngine { }; if(parent_data === undefined) { - this._widget_data.set(node_id, new_data); + widget.raw_widget_data.new_data; } else { parent_data[key_path[key_path.length-1]] = new_data; diff --git a/lona/client2/_lona/client2/widget.js b/lona/client2/_lona/client2/widget.js new file mode 100644 index 00000000..a14708c4 --- /dev/null +++ b/lona/client2/_lona/client2/widget.js @@ -0,0 +1,137 @@ +/* MIT License + +Copyright (c) 2020 Florian Scherf + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. + +*/ + +'use strict'; + +import { LonaWindowShim } from './window-shim.js'; + + +function deepCopy(object) { + return JSON.parse(JSON.stringify(object)); +} + + +class WidgetData { + constructor(widget) { + this._widget = widget; + + this.data = {}; + + this._update_data(); + } + + _update_data() { + this.data = deepCopy(this._widget.raw_widget_data); + } +} + + +export class Widget { + constructor( + lona_context, + lona_window, + root_node, + widget_id, + widget_class, + raw_widget_data, + ) { + + this.lona_context = lona_context; + this.lona_window = lona_window; + this.root_node = root_node; + this.widget_id = widget_id; + this.widget_class = widget_class; + this.raw_widget_data = raw_widget_data; + + this.window_shim = new LonaWindowShim( + this.lona_context, + this.lona_window, + this.widget_id, + ); + + this.widget_data = new WidgetData(this); + + this.widget_object = undefined; + } + + initialize_widget() { + this.widget_object = new this.widget_class( + this.window_shim, + this.root_node, + this.widget_data, + ); + + // legacy API + // TODO: remove in 2.0 + // set data + this.widget_object.data = this.widget_data.data; + + // set nodes + this.widget_object.root_node = this.root_node; + this.widget_object.nodes = [this.root_node]; + + // legacy setup hook + if(this.widget_object.setup !== undefined) { + this.widget_object.setup(); + } + } + + destroy_widget() { + if(this.widget_object === undefined) { + return; + } + + if(this.widget_object.destroy !== undefined) { + this.widget_object.destroy(); + } + + // legacy API + // TODO: remove in 2.0 + if(this.widget_object.deconstruct !== undefined) { + this.widget_object.deconstruct(); + } + } + + run_data_updated_hook() { + if(this.widget_object === undefined) { + return; + } + + this.widget_data._update_data(); + + if(this.widget_object.onDataUpdated !== undefined) { + this.widget_object.onDataUpdated(this.widget_data); + } + + // legacy API + // TODO: remove in 2.0 + this.widget_object.data = this.widget_data.data; + + if(this.widget_object.data_updated !== undefined) { + this.widget_object.data_updated(); + + return; + } + } +} From b5d0c5ecc2a50f0cdefdd28340bf48807f472974 Mon Sep 17 00:00:00 2001 From: Florian Scherf Date: Sat, 11 Mar 2023 23:19:01 +0100 Subject: [PATCH 10/18] client: use new widget API implementation of client2 This patch updates the client to use the widget API implementation of client2, in order to be compatible with the old and the new widget API, until Lona 2 gets released. Signed-off-by: Florian Scherf --- lona/client/_lona/client/dom-renderer.js | 22 ++++------ .../_lona/client/widget-data-updater.js | 8 ++-- lona/client/_lona/client/window.js | 41 ++++++------------- 3 files changed, 26 insertions(+), 45 deletions(-) diff --git a/lona/client/_lona/client/dom-renderer.js b/lona/client/_lona/client/dom-renderer.js index e8cadf3d..34b799a7 100644 --- a/lona/client/_lona/client/dom-renderer.js +++ b/lona/client/_lona/client/dom-renderer.js @@ -22,7 +22,7 @@ SOFTWARE. */ -import { LonaWindowShim } from './window-shim.js'; +import { Widget } from '../client2/widget.js'; import { Lona } from './lona.js'; @@ -123,18 +123,16 @@ export class LonaDomRenderer { throw(`RuntimeError: unknown widget name '${widget_class_name}'`); } - var widget_class = Lona.widget_classes[widget_class_name]; - - var window_shim = new LonaWindowShim( + const widget = new Widget( this.lona_context, this.lona_window, + node, node_id, + Lona.widget_classes[widget_class_name], + widget_data, ); - var widget = new widget_class(window_shim); - this.lona_window._widgets[node_id] = widget; - this.lona_window._widget_data[node_id] = widget_data; this.lona_window._widgets_to_setup.splice(0, 0, node_id); } @@ -179,18 +177,16 @@ export class LonaDomRenderer { // setup widget if(node_widget_class_name in Lona.widget_classes) { - var widget_class = Lona.widget_classes[node_widget_class_name]; - - var window_shim = new LonaWindowShim( + const widget = new Widget( this.lona_context, this.lona_window, + node, node_id, + Lona.widget_classes[widget_class_name], + widget_data, ); - var widget = new widget_class(window_shim); - this.lona_window._widgets[node_id] = widget; - this.lona_window._widget_data[node_id] = widget_data; this.lona_window._widgets_to_setup.splice(0, 0, node_id); }; }; diff --git a/lona/client/_lona/client/widget-data-updater.js b/lona/client/_lona/client/widget-data-updater.js index 63f9d114..953bfe14 100644 --- a/lona/client/_lona/client/widget-data-updater.js +++ b/lona/client/_lona/client/widget-data-updater.js @@ -40,9 +40,11 @@ export class LonaWidgetDataUpdater { var key_path = payload[0]; var data = payload.splice(1); + const widget = this.lona_window._widgets[node_id]; + // key path var parent_data = undefined; - var widget_data = this.lona_window._widget_data[node_id]; + let widget_data = widget.raw_widget_data; key_path.forEach(function(key) { parent_data = widget_data; @@ -56,7 +58,7 @@ export class LonaWidgetDataUpdater { // RESET } else if(operation == Lona.protocol.OPERATION.RESET) { if(parent_data === undefined) { - this.lona_window._widget_data[node_id] = data[0]; + widget.raw_widget_data = data[0]; } else { parent_data = data[0]; @@ -74,7 +76,7 @@ export class LonaWidgetDataUpdater { }; if(parent_data === undefined) { - this.lona_window._widget_data[node_id] = new_data; + widget.raw_widget_data = new_data; } else { parent_data[key_path[key_path.length-1]] = new_data; diff --git a/lona/client/_lona/client/window.js b/lona/client/_lona/client/window.js index dac46cb5..2045535d 100644 --- a/lona/client/_lona/client/window.js +++ b/lona/client/_lona/client/window.js @@ -25,7 +25,7 @@ SOFTWARE. import { LonaWidgetDataUpdater } from './widget-data-updater.js' import { LonaInputEventHandler } from './input-events.js'; import { LonaDomRenderer } from './dom-renderer.js'; -import { LonaDomUpdater } from './dom-updater.js' +import { LonaDomUpdater } from './dom-updater.js'; import { Lona } from './lona.js' @@ -120,9 +120,7 @@ export class LonaWindow { _clear_node_cache() { // running widget deconstructors for(var key in this._widgets) { - if(this._widgets[key].deconstruct !== undefined) { - this._widgets[key].deconstruct(); - }; + this._widgets[key].destroy_widget(); }; // resetting node state @@ -159,9 +157,7 @@ export class LonaWindow { if(key in this._widgets) { // run deconstructor - if(this._widgets[key].deconstruct !== undefined) { - this._widgets[key].deconstruct(); - }; + this._widgets[key].destroy_widget(); // remove widget delete this._widgets[key]; @@ -188,37 +184,24 @@ export class LonaWindow { _run_widget_hooks() { // setup this._widgets_to_setup.forEach(node_id => { - var widget = this._widgets[node_id]; - var widget_data = this._widget_data[node_id]; - - widget.data = JSON.parse(JSON.stringify(widget_data)); - - if(widget === undefined) { + if(this._widgets[node_id] === undefined) { return; - }; + } - widget.nodes = this._dom_updater._get_widget_nodes(node_id); - widget.root_node = widget.nodes[0]; + const widget = this._widgets[node_id]; - if(widget.setup !== undefined) { - widget.setup(); - }; + widget.initialize_widget(); }); // data_updated this._widgets_to_update.forEach(node_id => { - var widget = this._widgets[node_id]; - var widget_data = this._widget_data[node_id]; - - widget.data = JSON.parse(JSON.stringify(widget_data)); - - if(widget === undefined) { + if(this._widgets[node_id] === undefined) { return; - }; + } - if(widget.data_updated !== undefined) { - widget.data_updated(); - }; + const widget = this._widgets[node_id]; + + widget.run_data_updated_hook(); }); this._widgets_to_setup = []; From 1d819bf9ae984e356fb0b4965754cfe806c4c72d Mon Sep 17 00:00:00 2001 From: Florian Scherf Date: Fri, 17 Mar 2023 17:14:01 +0100 Subject: [PATCH 11/18] client2: window shim: add reference to Lona window root node This new property is helpful for global window actions, in case the server server disconnects, and for testing the widget API. Signed-off-by: Florian Scherf --- lona/client2/_lona/client2/window-shim.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lona/client2/_lona/client2/window-shim.js b/lona/client2/_lona/client2/window-shim.js index c38ff3ea..db5dde7e 100644 --- a/lona/client2/_lona/client2/window-shim.js +++ b/lona/client2/_lona/client2/window-shim.js @@ -28,6 +28,8 @@ export class LonaWindowShim { this._lona_window = lona_window; this._widget_id = widget_id; + + this.root_node = this._lona_window._root; }; fire_input_event(node, event_type, data, target_node) { From bfddedf2e56dc605a66a276c5f8317cf04d92984 Mon Sep 17 00:00:00 2001 From: Florian Scherf Date: Fri, 17 Mar 2023 18:40:10 +0100 Subject: [PATCH 12/18] client: use window shim implementation of client2 The window shim implementation of client2 will be the place for all abstraction of the old and the new widget API, to facilitate a smooth transition, from Lona 1 to Lona 2. This patch updates the client, to use the new implementation, as early as possible. Signed-off-by: Florian Scherf --- lona/client/_lona/client/context.js | 2 +- lona/client/_lona/client/window-shim.js | 49 ------------------------- 2 files changed, 1 insertion(+), 50 deletions(-) delete mode 100644 lona/client/_lona/client/window-shim.js diff --git a/lona/client/_lona/client/context.js b/lona/client/_lona/client/context.js index 986b4b99..a7b6520d 100644 --- a/lona/client/_lona/client/context.js +++ b/lona/client/_lona/client/context.js @@ -22,7 +22,7 @@ SOFTWARE. */ -import { LonaWindowShim } from './window-shim.js'; +import { LonaWindowShim } from '../client2/window-shim.js'; import { LonaWindow } from './window.js'; import { Lona } from './lona.js'; diff --git a/lona/client/_lona/client/window-shim.js b/lona/client/_lona/client/window-shim.js deleted file mode 100644 index c38ff3ea..00000000 --- a/lona/client/_lona/client/window-shim.js +++ /dev/null @@ -1,49 +0,0 @@ -/* MIT License - -Copyright (c) 2020 Florian Scherf - -Permission is hereby granted, free of charge, to any person obtaining a copy -of this software and associated documentation files (the "Software"), to deal -in the Software without restriction, including without limitation the rights -to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -copies of the Software, and to permit persons to whom the Software is -furnished to do so, subject to the following conditions: - -The above copyright notice and this permission notice shall be included in all -copies or substantial portions of the Software. - -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -SOFTWARE. - -*/ - -export class LonaWindowShim { - constructor(lona_context, lona_window, widget_id) { - this.lona_context = lona_context; - - this._lona_window = lona_window; - this._widget_id = widget_id; - }; - - fire_input_event(node, event_type, data, target_node) { - return this._lona_window._input_event_handler.fire_input_event( - node || this._widget_id, - event_type, - data, - target_node, - ); - }; - - set_html(html) { - if(this._lona_window._view_running) { - throw('RuntimeError: cannot set HTML while a view is running'); - }; - - this._lona_window._root.innerHTML = html; - }; -}; From 655c8629cf3602052f8f3176e41a685bb1355d71 Mon Sep 17 00:00:00 2001 From: Florian Scherf Date: Fri, 17 Mar 2023 23:01:41 +0100 Subject: [PATCH 13/18] client2: fix widget destroy hook Previously, `Widget.destroy` only got called when a node got orphaned by a node clearing operation, and collected by `LonaRenderingEngine._clean_node_cache`. It did not run, when a single node got removed from the client. This patch moves the destroy and clean-up code into a helper method of the rendering engine, and adds a call to it to the node removal code, so `Widget.destroy` gets called every time a node gets removed from the client. Signed-off-by: Florian Scherf --- .../client2/_lona/client2/rendering-engine.js | 56 ++++++++++--------- 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/lona/client2/_lona/client2/rendering-engine.js b/lona/client2/_lona/client2/rendering-engine.js index b6081403..ce28500e 100644 --- a/lona/client2/_lona/client2/rendering-engine.js +++ b/lona/client2/_lona/client2/rendering-engine.js @@ -101,8 +101,36 @@ export class LonaRenderingEngine { this._nodes.delete(node_id); node.remove(); + + this._remove_widget_if_present(node_id); }; + _remove_widget_if_present(node_id) { + if(!(this._widgets.has(node_id))) { + return; + } + + const widget = this._widgets.get(node_id); + + // destroy widget + widget.destroy_widget(); + + // remove widget + this._widgets.delete(node_id); + + // remove widget from _widgets_to_setup + if(this._widgets_to_setup.indexOf(node_id) > -1) { + this._widgets_to_setup.splice( + this._widgets_to_setup.indexOf(node_id), 1); + }; + + // remove widget from _widgets_to_update + if(this._widgets_to_update.indexOf(node_id) > -1) { + this._widgets_to_update.splice( + this._widgets_to_update.indexOf(node_id), 1); + }; + } + _clear_node(node_id) { const node = this._get_node(node_id); @@ -132,38 +160,12 @@ export class LonaRenderingEngine { _clean_node_cache() { this._nodes.forEach((node, node_id) => { - - // nodes if(this._root.contains(node)) { return; } this._remove_node(node_id); - - // widgets - if(!(this._widgets.has(node_id))) { - return; - } - - const widget = this._widgets.get(node_id); - - // destroy widget - widget.destroy_widget(); - - // remove widget - this._widgets.delete(node_id); - - // remove widget from _widgets_to_setup - if(this._widgets_to_setup.indexOf(node_id) > -1) { - this._widgets_to_setup.splice( - this._widgets_to_setup.indexOf(node_id), 1); - }; - - // remove widget from _widgets_to_update - if(this._widgets_to_update.indexOf(node_id) > -1) { - this._widgets_to_update.splice( - this._widgets_to_update.indexOf(node_id), 1); - }; + this._remove_widget_if_present(node_id); }); }; From e50a8a194c701fd43e5254c7a681e57ae4820c80 Mon Sep 17 00:00:00 2001 From: Florian Scherf Date: Sat, 18 Mar 2023 11:55:17 +0100 Subject: [PATCH 14/18] client: fix widget deconstruct hook Previously, `Widget.deconstruct` only got called when a node got orphaned by a node clearing operation, and collected by `_clean_node_cache`. It did not run, when a singe node got removed from the client. This patch moves the deconstruct and clean-up code into a helper method, and adds a call to it to the node removal code, so `Widget.deconstruct` gets called every time a node gets removed from the client. Signed-off-by: Florian Scherf --- lona/client/_lona/client/dom-updater.js | 13 +++--- lona/client/_lona/client/window.js | 53 ++++++++++++++----------- 2 files changed, 35 insertions(+), 31 deletions(-) diff --git a/lona/client/_lona/client/dom-updater.js b/lona/client/_lona/client/dom-updater.js index be61806e..cbeb294c 100644 --- a/lona/client/_lona/client/dom-updater.js +++ b/lona/client/_lona/client/dom-updater.js @@ -275,9 +275,12 @@ export class LonaDomUpdater { _remove_node(node_id) { - // TextNode + // Node if(node_id in this.lona_window._nodes) { - this.lona_window._nodes[node_id].remove(); + node = this.lona_window._nodes[node_id]; + + node.remove(); + this.lona_window._remove_widget_if_present(node_id); delete this.lona_window._nodes[node_id]; @@ -310,11 +313,7 @@ export class LonaDomUpdater { node.remove(); }; - // Node - } else { - node = this.lona_window._nodes[node_id]; - - node.remove(); + this.lona_window._remove_widget_if_present(node_id); }; }; diff --git a/lona/client/_lona/client/window.js b/lona/client/_lona/client/window.js index 2045535d..4f965113 100644 --- a/lona/client/_lona/client/window.js +++ b/lona/client/_lona/client/window.js @@ -132,6 +132,33 @@ export class LonaWindow { this._widgets_to_update = []; }; + _remove_widget_if_present(node_id) { + if(!(node_id in this._widgets)) { + return; + } + + // run deconstructor + this._widgets[node_id].destroy_widget(); + + // remove widget + delete this._widgets[node_id]; + + // remove widget data + delete this._widget_data[node_id]; + + // remove widget from _widgets_to_setup + if(this._widgets_to_setup.indexOf(node_id) > -1) { + this._widgets_to_setup.splice( + this._widgets_to_setup.indexOf(node_id), 1); + }; + + // remove widget from _widgets_to_update + if(this._widgets_to_update.indexOf(node_id) > -1) { + this._widgets_to_update.splice( + this._widgets_to_update.indexOf(node_id), 1); + }; + } + _clean_node_cache() { // nodes Object.keys(this._nodes).forEach(key => { @@ -153,30 +180,8 @@ export class LonaWindow { delete this._widget_marker[key]; - // widget - if(key in this._widgets) { - - // run deconstructor - this._widgets[key].destroy_widget(); - - // remove widget - delete this._widgets[key]; - - // remove widget data - delete this._widget_data[key]; - - // remove widget from _widgets_to_setup - if(this._widgets_to_setup.indexOf(key) > -1) { - this._widgets_to_setup.splice( - this._widgets_to_setup.indexOf(key), 1); - }; - - // remove widget from _widgets_to_update - if(this._widgets_to_update.indexOf(key) > -1) { - this._widgets_to_update.splice( - this._widgets_to_update.indexOf(key), 1); - }; - }; + // frontend widget + this._remove_widget_if_present(key); }); }; From 5ada40268de595b8223ffe20a7e7860a52346976 Mon Sep 17 00:00:00 2001 From: Florian Scherf Date: Tue, 14 Mar 2023 20:53:17 +0100 Subject: [PATCH 15/18] tests: rendering: add tests for both widget APIs Signed-off-by: Florian Scherf --- .../views/frontend/rendering-test-widgets.js | 87 +++++++- test_project/views/frontend/rendering.py | 188 +++++++++++++++--- tests/test_rendering.py | 51 ++++- 3 files changed, 282 insertions(+), 44 deletions(-) diff --git a/test_project/views/frontend/rendering-test-widgets.js b/test_project/views/frontend/rendering-test-widgets.js index 975f30fc..96a81075 100644 --- a/test_project/views/frontend/rendering-test-widgets.js +++ b/test_project/views/frontend/rendering-test-widgets.js @@ -1,23 +1,40 @@ -class WidgetDataTestWidget { - constructor(lona_window) { - this.lona_window = lona_window; - } +class LegacyWidgetApiTestWidget { + // TODO: remove in 2.0 + // helper ----------------------------------------------------------------- render() { this.root_node.children[1].innerHTML = JSON.stringify(this.data); } + log_hook(name) { + const element = this.lona_window.root_node.querySelector( + '#widget-hooks', + ); + + if(!element) { + return; + } + + if(element.innerHTML != '') { + element.innerHTML = `${element.innerHTML},`; + } + + element.innerHTML = `${element.innerHTML}${name}`; + } + + // hooks ------------------------------------------------------------------ + constructor(lona_window) { + this.lona_window = lona_window; + this.log_hook('constructor'); + } + setup() { this.render(); - console.log('>> setup', this.nodes); + this.log_hook('setup'); } deconstruct() { - console.log('>> deconstruct', this.nodes); - } - - nodes_updated() { - console.log('>> nodes updated', this.nodes); + this.log_hook('deconstruct'); } data_updated() { @@ -26,6 +43,49 @@ class WidgetDataTestWidget { } +class WidgetApiTestWidget { + + // helper ----------------------------------------------------------------- + render(data) { + this.rootNode.children[1].innerHTML = JSON.stringify(data); + } + + log_hook(name) { + const element = this.lonaWindow.root_node.querySelector( + '#widget-hooks', + ); + + if(!element) { + return; + } + + if(element.innerHTML != '') { + element.innerHTML = `${element.innerHTML},`; + } + + element.innerHTML = `${element.innerHTML}${name}`; + } + + // hooks ------------------------------------------------------------------ + constructor(lonaWindow, rootNode, widgetData) { + this.lonaWindow = lonaWindow; + this.rootNode = rootNode; + this.widgetData = widgetData; + + this.render(this.widgetData.data); + this.log_hook('constructor'); + } + + destroy() { + this.log_hook('destroy'); + } + + onDataUpdated(widgetData) { + this.render(widgetData.data); + } +} + + class HTMLConsoleWidget { constructor(lona_window) { this.lona_window = lona_window; @@ -88,5 +148,10 @@ class HTMLConsoleWidget { } -Lona.register_widget_class('WidgetDataTestWidget', WidgetDataTestWidget); +Lona.register_widget_class( + 'LegacyWidgetApiTestWidget', + LegacyWidgetApiTestWidget, +); + +Lona.register_widget_class('WidgetApiTestWidget', WidgetApiTestWidget); Lona.register_widget_class('HTMLConsoleWidget', HTMLConsoleWidget); diff --git a/test_project/views/frontend/rendering.py b/test_project/views/frontend/rendering.py index 06f755eb..98ea4139 100644 --- a/test_project/views/frontend/rendering.py +++ b/test_project/views/frontend/rendering.py @@ -28,8 +28,8 @@ def decorator(step): return decorator -class WidgetDataTestComponent(Div): - WIDGET = 'WidgetDataTestWidget' +class LegacyWidgetApiTestComponent(Div): + WIDGET = 'LegacyWidgetApiTestWidget' def __init__(self, initial_state): super().__init__() @@ -50,6 +50,10 @@ def update_state(self): self.server_state.set_text(dumps(self.widget_data)) +class WidgetApiTestComponent(LegacyWidgetApiTestComponent): + WIDGET = 'WidgetApiTestWidget' + + class HTMLConsole(Div): WIDGET = 'HTMLConsoleWidget' @@ -505,20 +509,21 @@ def step_28(self): self.rendering_root.nodes[0].style.clear() - # widget data tests + # legacy widget api tests @client_version(1, 2) def step_29(self): - self.set_step_label(29, 'Widget Data: list: setup') + self.set_step_label(29, 'Legacy Widget API: setup') self.rendering_root.nodes = [ - WidgetDataTestComponent( + LegacyWidgetApiTestComponent( initial_state={'list': []}, ), + Div(id='widget-hooks'), ] @client_version(1, 2) def step_30(self): - self.set_step_label(30, 'Widget Data: list: append') + self.set_step_label(30, 'Legacy Widget API: data: list: append') component = self.rendering_root.nodes[0] @@ -529,7 +534,7 @@ def step_30(self): @client_version(1, 2) def step_31(self): - self.set_step_label(31, 'Widget Data: list: remove') + self.set_step_label(31, 'Legacy Widget API: data: list: remove') component = self.rendering_root.nodes[0] @@ -538,7 +543,7 @@ def step_31(self): @client_version(1, 2) def step_32(self): - self.set_step_label(32, 'Widget Data: list: insert') + self.set_step_label(32, 'Legacy Widget API: data: list: insert') component = self.rendering_root.nodes[0] @@ -547,7 +552,7 @@ def step_32(self): @client_version(1, 2) def step_33(self): - self.set_step_label(33, 'Widget Data: list: clear') + self.set_step_label(33, 'Legacy Widget API: data: list: clear') component = self.rendering_root.nodes[0] @@ -556,7 +561,7 @@ def step_33(self): @client_version(1, 2) def step_34(self): - self.set_step_label(34, 'Widget Data: list: reset') + self.set_step_label(34, 'Legacy Widget API: data: list: reset') component = self.rendering_root.nodes[0] @@ -565,7 +570,7 @@ def step_34(self): @client_version(1, 2) def step_35(self): - self.set_step_label(35, 'Widget Data: dict: setup') + self.set_step_label(35, 'Legacy Widget API: data: dict: setup') component = self.rendering_root.nodes[0] @@ -574,7 +579,7 @@ def step_35(self): @client_version(1, 2) def step_36(self): - self.set_step_label(36, 'Widget Data: dict: set') + self.set_step_label(36, 'Legacy Widget API: data: dict: set') component = self.rendering_root.nodes[0] @@ -585,7 +590,7 @@ def step_36(self): @client_version(1, 2) def step_37(self): - self.set_step_label(37, 'Widget Data: dict: del') + self.set_step_label(37, 'Legacy Widget API: data: dict: del') component = self.rendering_root.nodes[0] @@ -594,7 +599,7 @@ def step_37(self): @client_version(1, 2) def step_38(self): - self.set_step_label(38, 'Widget Data: dict: pop') + self.set_step_label(38, 'Legacy Widget API: data: dict: pop') component = self.rendering_root.nodes[0] @@ -603,7 +608,7 @@ def step_38(self): @client_version(1, 2) def step_39(self): - self.set_step_label(39, 'Widget Data: dict: clear') + self.set_step_label(39, 'Legacy Widget API: data: dict: clear') component = self.rendering_root.nodes[0] @@ -612,19 +617,146 @@ def step_39(self): @client_version(1, 2) def step_40(self): - self.set_step_label(40, 'Widget Data: dict: reset') + self.set_step_label(40, 'Legacy Widget API: data: dict: reset') + + component = self.rendering_root.nodes[0] + + component.widget_data['dict'] = {0: 0, 1: 1, 2: 2, 3: 3, 4: 4} + component.update_state() + + @client_version(1, 2) + def step_41(self): + self.set_step_label(41, 'Legacy Widget API: destroy') + + self.rendering_root.nodes.pop(0) + + # widget api tests + @client_version(1, 2) + def step_42(self): + self.set_step_label(42, 'Widget API: setup') + + self.rendering_root.nodes = [ + WidgetApiTestComponent( + initial_state={'list': []}, + ), + Div(id='widget-hooks'), + ] + + @client_version(1, 2) + def step_43(self): + self.set_step_label(43, 'Widget API: data: list: append') + + component = self.rendering_root.nodes[0] + + component.widget_data['list'].append(1) + component.widget_data['list'].append(2) + component.widget_data['list'].append(3) + component.update_state() + + @client_version(1, 2) + def step_44(self): + self.set_step_label(44, 'Widget API: data: list: remove') + + component = self.rendering_root.nodes[0] + + component.widget_data['list'].remove(2) + component.update_state() + + @client_version(1, 2) + def step_45(self): + self.set_step_label(45, 'Widget API: data: list: insert') + + component = self.rendering_root.nodes[0] + + component.widget_data['list'].insert(0, 0) + component.update_state() + + @client_version(1, 2) + def step_46(self): + self.set_step_label(46, 'Widget API: data: list: clear') + + component = self.rendering_root.nodes[0] + + component.widget_data['list'].clear() + component.update_state() + + @client_version(1, 2) + def step_47(self): + self.set_step_label(47, 'Widget API: data: list: reset') + + component = self.rendering_root.nodes[0] + + component.widget_data['list'] = [5, 4, 3, 2, 1] + component.update_state() + + @client_version(1, 2) + def step_48(self): + self.set_step_label(48, 'Widget API: data: dict: setup') + + component = self.rendering_root.nodes[0] + + component.widget_data = {'dict': {}} + component.update_state() + + @client_version(1, 2) + def step_49(self): + self.set_step_label(49, 'Widget API: data: dict: set') + + component = self.rendering_root.nodes[0] + + component.widget_data['dict'][1] = 1 + component.widget_data['dict'][2] = 2 + component.widget_data['dict'][3] = 3 + component.update_state() + + @client_version(1, 2) + def step_50(self): + self.set_step_label(50, 'Widget API: data: dict: del') + + component = self.rendering_root.nodes[0] + + del component.widget_data['dict'][2] + component.update_state() + + @client_version(1, 2) + def step_51(self): + self.set_step_label(51, 'Widget API: data: dict: pop') + + component = self.rendering_root.nodes[0] + + component.widget_data['dict'].pop(3) + component.update_state() + + @client_version(1, 2) + def step_52(self): + self.set_step_label(52, 'Widget API: data: dict: clear') + + component = self.rendering_root.nodes[0] + + component.widget_data['dict'].clear() + component.update_state() + + @client_version(1, 2) + def step_53(self): + self.set_step_label(53, 'Widget API: data: dict: reset') component = self.rendering_root.nodes[0] component.widget_data['dict'] = {0: 0, 1: 1, 2: 2, 3: 3, 4: 4} component.update_state() + @client_version(1, 2) + def step_54(self): + self.set_step_label(54, 'Widget API: destroy') + + self.rendering_root.nodes.pop(0) + # legacy widgets ########################################################## # TODO: remove in 2.0 @client_version(1) - def step_41(self): - self.set_step_label(41, 'Legacy Widgets: Setup') + def step_55(self): + self.set_step_label(55, 'Legacy Widgets: Setup') self.rendering_root.clear() @@ -641,8 +773,8 @@ def step_41(self): ] @client_version(1) - def step_42(self): - self.set_step_label(42, 'Legacy Widgets: Append Nodes') + def step_56(self): + self.set_step_label(56, 'Legacy Widgets: Append Nodes') widget1 = self.rendering_root.nodes[0] widget1.append(Div('1.3')) @@ -653,8 +785,8 @@ def step_42(self): self.rendering_root.append(Div('4.1')) @client_version(1) - def step_43(self): - self.set_step_label(43, 'Legacy Widgets: Set Nodes') + def step_57(self): + self.set_step_label(57, 'Legacy Widgets: Set Nodes') widget1 = self.rendering_root.nodes[0] widget1.nodes[1] = Div('1.2.1') @@ -663,8 +795,8 @@ def step_43(self): widget1.nodes[1] = Div('3.2.1') @client_version(1) - def step_44(self): - self.set_step_label(44, 'Legacy Widgets: Reset Nodes') + def step_58(self): + self.set_step_label(58, 'Legacy Widgets: Reset Nodes') widget1 = self.rendering_root.nodes[0] @@ -687,8 +819,8 @@ def step_44(self): self.rendering_root[3] = Div('4.1.1') @client_version(1) - def step_45(self): - self.set_step_label(45, 'Legacy Widgets: Insert Nodes') + def step_59(self): + self.set_step_label(59, 'Legacy Widgets: Insert Nodes') widget1 = self.rendering_root[0] widget1.nodes.insert(2, Div('1.2.1.1')) @@ -699,8 +831,8 @@ def step_45(self): widget2.nodes.insert(2, Div('3.2.1.1')) @client_version(1) - def step_46(self): - self.set_step_label(46, 'Legacy Widgets: Remove Nodes') + def step_60(self): + self.set_step_label(60, 'Legacy Widgets: Remove Nodes') widget1 = self.rendering_root[0] widget1.nodes.pop(2) diff --git a/tests/test_rendering.py b/tests/test_rendering.py index 518a79fc..9a48e1cf 100644 --- a/tests/test_rendering.py +++ b/tests/test_rendering.py @@ -97,6 +97,12 @@ async def parse_json(page, locator): return json.loads(json_string) + async def get_widget_hooks(page): + element = page.locator('#lona #rendering-root #widget-hooks') + widget_hooks = await element.inner_html() + + return widget_hooks.strip() + async with async_playwright() as p: browser = await getattr(p, browser_name).launch() browser_context = await browser.new_context() @@ -188,29 +194,64 @@ async def parse_json(page, locator): await next_step(page, 28) await check_default_styles(page) - # widget data tests ################################################### + # legacy widget API ################################################### + # TODO: remove in 2.0 for step in range(29, 41): await next_step(page, step) + # widget hooks + assert (await get_widget_hooks(page)) == 'constructor,setup' + + # widget data server_widget_data = await parse_json( page, - '#lona #server-widget-data', + '#lona #rendering-root #server-widget-data', ) client_widget_data = await parse_json( page, - '#lona #client-widget-data', + '#lona #rendering-root #server-widget-data', ) assert server_widget_data == client_widget_data - # legacy widgets tests ################################################ + # destroy + await next_step(page, 41) + + assert (await get_widget_hooks(page)) == 'constructor,setup,deconstruct' + + # widget API ########################################################## + for step in range(42, 54): + await next_step(page, step) + + # widget hooks + assert (await get_widget_hooks(page)) == 'constructor' + + # widget data + server_widget_data = await parse_json( + page, + '#lona #rendering-root #server-widget-data', + ) + + client_widget_data = await parse_json( + page, + '#lona #rendering-root #server-widget-data', + ) + + assert server_widget_data == client_widget_data + + # destroy + await next_step(page, 54) + + assert (await get_widget_hooks(page)) == 'constructor,destroy' + + # legacy frontend widgets tests ####################################### # TODO: remove in 2.0 if get_client_version() != 1: return - for step in range(41, 47): + for step in range(55, 61): await next_step(page, step) client_html_string = await rendering_root_element.inner_html() From e566fa73f71c839c2a2b0ede038c8acb926c65d2 Mon Sep 17 00:00:00 2001 From: Florian Scherf Date: Wed, 15 Mar 2023 22:09:03 +0100 Subject: [PATCH 16/18] client2: fix resolving of relative URLs in redirects Previously, the resolving of relative URLs was a custom implementation and had multiple weird quirks, and behaved differently than the browsers implementation. That was confusing, because redirects, issued by Lona, sometimes resulted in slightly different URLs than their link-counterpart. This patch updates the client to use the browsers implementation of URL resolving. Signed-off-by: Florian Scherf --- lona/client2/_lona/client2/window.js | 46 +++++----------------------- 1 file changed, 7 insertions(+), 39 deletions(-) diff --git a/lona/client2/_lona/client2/window.js b/lona/client2/_lona/client2/window.js index 9d68ca78..27df9137 100644 --- a/lona/client2/_lona/client2/window.js +++ b/lona/client2/_lona/client2/window.js @@ -48,46 +48,12 @@ export class LonaWindow { this._crashed = false; this._view_running = false; this._view_runtime_id = undefined; - this._url = ''; + this._url = new URL(window.location); }; // urls ------------------------------------------------------------------- _set_url(raw_url) { - // parse pathname, search and hash - var _raw_url = new URL(raw_url, window.location.origin); - - _raw_url = _raw_url.pathname + _raw_url.search + _raw_url.hash; - - if(raw_url.startsWith('..')) { - _raw_url = '..' + _raw_url; - - } else if(raw_url.startsWith('.')) { - _raw_url = '.' + _raw_url; - - }; - - if(!raw_url.startsWith('http://') && !raw_url.startsWith('https://')) { - if(_raw_url.startsWith('/') && !raw_url.startsWith('/')) { - _raw_url = _raw_url.substr(1); - } - }; - - raw_url = _raw_url; - - // handle relative URLs - if(!raw_url.startsWith('/')) { - var current_url = this._url; - - if(!current_url.endsWith('/')) { - current_url = current_url + '/'; - }; - - raw_url = current_url + raw_url; - }; - - var url = new URL(raw_url, window.location.origin); - - this._url = url.pathname + url.search + url.hash; + this._url = new URL(raw_url, this._url); }; get_url() { @@ -139,7 +105,7 @@ export class LonaWindow { this._view_running = true; if(this.lona_context.settings.update_address_bar) { - history.pushState({}, '', this.get_url()); + history.pushState({}, '', this.get_url().href); }; this._rendering_engine._clear(); @@ -163,7 +129,9 @@ export class LonaWindow { // http redirect } else if(method == Lona.protocol.METHOD.HTTP_REDIRECT) { if(this.lona_context.settings.follow_http_redirects) { - window.location = payload; + this._set_url(payload); + + window.location = this.get_url().href; } else { console.debug( @@ -254,7 +222,7 @@ export class LonaWindow { this._window_id, this._view_runtime_id, Lona.protocol.METHOD.VIEW, - [this.get_url(), post_data], + [this.get_url().href, post_data], ]; // update html title From 91dc555dde0c92ce2b39ad822ea21271afd8169d Mon Sep 17 00:00:00 2001 From: Florian Scherf Date: Wed, 15 Mar 2023 22:11:20 +0100 Subject: [PATCH 17/18] client: fix resolving of relative URLs in redirects Previously, the resolving of relative URLs was a custom implementation and had multiple weird quirks, and behaved differently than the browsers implementation. That was confusing, because redirects, issued by Lona, sometimes resulted in slightly different URLs than their link-counterpart. This patch updates the client to use the browsers implementation of URL resolving. Signed-off-by: Florian Scherf --- lona/client/_lona/client/window.js | 46 +++++------------------------- 1 file changed, 7 insertions(+), 39 deletions(-) diff --git a/lona/client/_lona/client/window.js b/lona/client/_lona/client/window.js index 4f965113..e973d3eb 100644 --- a/lona/client/_lona/client/window.js +++ b/lona/client/_lona/client/window.js @@ -59,7 +59,7 @@ export class LonaWindow { this._crashed = false; this._view_running = false; this._view_runtime_id = undefined; - this._url = ''; + this._url = new URL(window.location); this._nodes = {}; this._widget_marker = {}; this._widget_data = {}; @@ -70,41 +70,7 @@ export class LonaWindow { // urls ------------------------------------------------------------------- _set_url(raw_url) { - // parse pathname, search and hash - var _raw_url = new URL(raw_url, window.location.origin); - - _raw_url = _raw_url.pathname + _raw_url.search + _raw_url.hash; - - if(raw_url.startsWith('..')) { - _raw_url = '..' + _raw_url; - - } else if(raw_url.startsWith('.')) { - _raw_url = '.' + _raw_url; - - }; - - if(!raw_url.startsWith('http://') && !raw_url.startsWith('https://')) { - if(_raw_url.startsWith('/') && !raw_url.startsWith('/')) { - _raw_url = _raw_url.substr(1); - } - }; - - raw_url = _raw_url; - - // handle relative URLs - if(!raw_url.startsWith('/')) { - var current_url = this._url; - - if(!current_url.endsWith('/')) { - current_url = current_url + '/'; - }; - - raw_url = current_url + raw_url; - }; - - var url = new URL(raw_url, window.location.origin); - - this._url = url.pathname + url.search + url.hash; + this._url = new URL(raw_url, this._url); }; get_url() { @@ -306,7 +272,7 @@ export class LonaWindow { this._view_running = true; if(this.lona_context.settings.update_address_bar) { - history.pushState({}, '', this.get_url()); + history.pushState({}, '', this.get_url().href); }; this._clear(); @@ -330,7 +296,9 @@ export class LonaWindow { // http redirect } else if(method == Lona.protocol.METHOD.HTTP_REDIRECT) { if(this.lona_context.settings.follow_http_redirects) { - window.location = payload; + this._set_url(payload); + + window.location = this.get_url().href; } else { console.debug( @@ -421,7 +389,7 @@ export class LonaWindow { this._window_id, this._view_runtime_id, Lona.protocol.METHOD.VIEW, - [this.get_url(), post_data], + [this.get_url().href, post_data], ]; // update html title From 12158421969f37da5896f669756cb84dc8a4e5e8 Mon Sep 17 00:00:00 2001 From: Florian Scherf Date: Wed, 15 Mar 2023 17:18:34 +0100 Subject: [PATCH 18/18] tests: frontend: update redirect tests This patch updates the redirect test, to ensure relative link targets and relative redirects behave the same in both client implementations. Signed-off-by: Florian Scherf --- test_project/routes.py | 3 + test_project/views/frontend/redirects.py | 59 ++++++++ test_project/views/home.py | 1 + tests/test_redirects.py | 171 ++++++++++------------- 4 files changed, 134 insertions(+), 100 deletions(-) create mode 100644 test_project/views/frontend/redirects.py diff --git a/test_project/routes.py b/test_project/routes.py index 06d264c2..c61bd1f3 100644 --- a/test_project/routes.py +++ b/test_project/routes.py @@ -241,6 +241,9 @@ Route('/frontend/custom-messages/', 'views/frontend/custom_messages.py::CustomMessagesView'), + Route('/frontend/redirects', + 'views/frontend/redirects.py::RedirectsView'), + # home Route('/', 'views/home.py::HomeView'), ] diff --git a/test_project/views/frontend/redirects.py b/test_project/views/frontend/redirects.py new file mode 100644 index 00000000..bfcb1ed4 --- /dev/null +++ b/test_project/views/frontend/redirects.py @@ -0,0 +1,59 @@ +from lona.html import TextInput, Button, Label, HTML, Div, H2, A +from lona import HttpRedirectResponse, RedirectResponse, View + + +class RedirectsView(View): + def handle_url_change(self, input_event): + self.link.set_href(self.text_input.value) + + def redirect(self, input_event): + return RedirectResponse( + self.text_input.value, + ) + + def http_redirect(self, input_event): + return HttpRedirectResponse( + self.text_input.value, + ) + + def handle_request(self, request): + self.link = A( + 'Link', + href='#', + id='link', + ignore=True, + ) + + self.text_input = TextInput( + placeholder='URL', + id='url', + handle_change=self.handle_url_change, + ) + + self.html = HTML( + H2('Redirects'), + Div( + Label( + 'URL: ', + self.text_input, + ), + Button( + 'Redirect', + id='redirect', + handle_click=self.redirect, + ), + Button( + 'HTTP Redirect', + id='http-redirect', + handle_click=self.http_redirect, + ), + ), + Div( + Label( + 'Link: ', + self.link, + ), + ), + ) + + return self.html diff --git a/test_project/views/home.py b/test_project/views/home.py index 6b9c22da..126342eb 100644 --- a/test_project/views/home.py +++ b/test_project/views/home.py @@ -91,5 +91,6 @@ def handle_request(self, request):
  • Rendering
  • Custom Event
  • Custom Messages
  • +
  • Redirects
  • """ # NOQA: LN002 diff --git a/tests/test_redirects.py b/tests/test_redirects.py index edfc8387..1c365b1a 100644 --- a/tests/test_redirects.py +++ b/tests/test_redirects.py @@ -1,106 +1,77 @@ -def setup_app(app): - from lona import View - - # root - @app.route('/') - class RootView(View): - def handle_request(self, request): - return 'ROOT' - - @app.route('/redirect-to-root/') - class RedirectToRoot(View): - def handle_request(self, request): - return { - 'redirect': '/', - } - - # absolute url - @app.route('/absolute-url/') - class AbsoluteUrlView(View): - def handle_request(self, request): - return 'ABSOLUTE-URL' - - @app.route('/redirect-to-absolute-url/') - class RedirectToAbsoluteUrlView(View): - def handle_request(self, request): - return { - 'redirect': '/absolute-url/', - } - - # relative urls - @app.route('/redirect-to-relative-url/foo/') - class RelativeUrlView(View): - def handle_request(self, request): - return 'relative-URL' - - @app.route('/redirect-to-relative-url/') - class RelativeRedirectUrlView(View): - def handle_request(self, request): - return { - 'redirect': 'foo/', - } - - @app.route('/redirect-to-root-relatively/') - class RedirectToRootRelativeView(View): - def handle_request(self, request): - return { - 'redirect': '..', - } - - # refresh - refreshed = False - - @app.route('/refresh/') - class RefreshView(View): - def handle_request(self, request): - nonlocal refreshed - - if not refreshed: - refreshed = True - - return { - 'redirect': '.', - } - - else: - return 'REFRESH' - - -async def test_redirects(lona_app_context): - """ - This test tests redirects by creating multiple views and redirecting from - one to another. Absolute and relative URLs are tested. - """ +import pytest + + +@pytest.mark.parametrize('client_version', [1, 2]) +@pytest.mark.parametrize('browser_name', ['chromium', 'firefox', 'webkit']) +@pytest.mark.parametrize('method', ['link', 'redirect', 'http-redirect']) +async def test_redirects( + method, + browser_name, + client_version, + lona_project_context, +): + + import os from playwright.async_api import async_playwright - context = await lona_app_context(setup_app) + TEST_PROJECT_PATH = os.path.join(__file__, '../../test_project') + BASE_URL = '/frontend/redirects' + + context = await lona_project_context( + project_root=TEST_PROJECT_PATH, + settings=['settings.py'], + settings_post_overrides={ + 'CLIENT_VERSION': client_version, + }, + ) + + datasets = [ + + # absolute url + (f'{BASE_URL}/foo/bar/baz/', '/foo', '/foo'), + + # relative urls + (f'{BASE_URL}/foo/bar/baz', '.', f'{BASE_URL}/foo/bar/'), + (f'{BASE_URL}/foo/bar/baz/', '.', f'{BASE_URL}/foo/bar/baz/'), + + # relative forward urls + (f'{BASE_URL}/foo/bar/baz', './foo', f'{BASE_URL}/foo/bar/foo'), + (f'{BASE_URL}/foo/bar/baz/', './foo', f'{BASE_URL}/foo/bar/baz/foo'), + + # relative backward urls + (f'{BASE_URL}/foo/bar/baz', '..', f'{BASE_URL}/foo/'), + (f'{BASE_URL}/foo/bar/baz/', '..', f'{BASE_URL}/foo/bar/'), + ] async with async_playwright() as p: - browser = await p.chromium.launch() + browser = await getattr(p, browser_name).launch() browser_context = await browser.new_context() - page = await browser_context.new_page() - - # test redirect to root - await page.goto(context.make_url('/redirect-to-root/')) - await page.wait_for_url('/') - await page.wait_for_selector('#lona:has-text("ROOT")') - - # test redirect to absolute url - await page.goto(context.make_url('/redirect-to-absolute-url/')) - await page.wait_for_url('/absolute-url/') - await page.wait_for_selector('#lona:has-text("ABSOLUTE-URL")') - - # relative url - await page.goto(context.make_url('/redirect-to-relative-url/')) - await page.wait_for_url('/redirect-to-relative-url/foo/') - await page.wait_for_selector('#lona:has-text("RELATIVE-URL")') - - await page.goto(context.make_url('/redirect-to-root-relatively/')) - await page.wait_for_url('/') - await page.wait_for_selector('#lona:has-text("ROOT")') - - # test refresh - await page.goto(context.make_url('/refresh/')) - await page.wait_for_url('/refresh/') - await page.wait_for_selector('#lona:has-text("REFRESH")') + + for raw_initial_url, redirect_url, success_url in datasets: + page = await browser_context.new_page() + + # initial load + initial_url = context.make_url(raw_initial_url) + + await page.goto(initial_url) + await page.wait_for_url(initial_url) + + # trigger redirect + await page.fill('input#url', redirect_url) + await page.wait_for_selector(f'a#link[href="{redirect_url}"]') + + if method == 'link': + await page.click('a#link') + + elif method == 'redirect': + await page.click('button#redirect') + + elif method == 'http-redirect': + await page.click('button#http-redirect') + + # wait for success url + await page.wait_for_url(success_url) + + # close page + await page.close()