From ea17b036f1425e5d21a0c0fb535f876a0141d585 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Wed, 18 Dec 2024 12:36:49 +0100 Subject: [PATCH 01/17] =?UTF-8?q?=F0=9F=A7=B9=20Add=20checks=20for=20dupli?= =?UTF-8?q?cate=20HTML=20element=20IDs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the course of investigating other issues, I've found that we are emitting invalid HTML. For example, multiple elements with the same `id="..."` attribute. This doesn't cause loud errors in the browser, but interferes with the proper functioning HTML selectors and Cypress tests don't always behave like they should. Add the start of a postprocessing step where we are going to validate the HTML that is generated in Flask templates and crash loudly if it is illegal. --- app.py | 2 ++ website/flask_helpers.py | 13 +++++++++++-- website/html_validation.py | 13 +++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) create mode 100644 website/html_validation.py diff --git a/app.py b/app.py index 8158d2790d2..daebea7e8ce 100644 --- a/app.py +++ b/app.py @@ -181,6 +181,8 @@ def load_all_adventures_for_index(customizations, subset=None): return all_adventures +from flask import template_rendered + def load_adventures_for_level(level, subset=None): """Load the adventures available to the current user at the given level. diff --git a/website/flask_helpers.py b/website/flask_helpers.py index 87e3a01b558..248f01a1453 100644 --- a/website/flask_helpers.py +++ b/website/flask_helpers.py @@ -13,8 +13,17 @@ @querylog.timed def render_template(filename, **kwargs): - """A copy of Flask's render_template that is timed.""" - return flask.render_template(filename, **kwargs) + """A copy of Flask's render_template that is timed, and also validated.""" + rendered = flask.render_template(filename, **kwargs) + + # Late imports because of circular dependencies. Don't care to figure this out right now. + from utils import is_debug_mode + if is_debug_mode(): + from website.html_validation import validate_output_html + validate_output_html(rendered) + return rendered + + def proper_json_dumps(x, **kwargs): diff --git a/website/html_validation.py b/website/html_validation.py new file mode 100644 index 00000000000..33a6d9c72c7 --- /dev/null +++ b/website/html_validation.py @@ -0,0 +1,13 @@ +import re +from collections import Counter + +def validate_output_html(html): + validate_no_duplicate_ids(html) + + +ID_RE = re.compile('id="([^"]+)"') + +def validate_no_duplicate_ids(html): + ids = Counter(ID_RE.findall(html)) + duplicates = [id for id, count in ids.items() if count > 1] + assert not duplicates, 'There are multiple HTML elements with the same id="..." on this page. IDs should be unique: %s' % ', '.join(duplicates) \ No newline at end of file From 03bd03ec28ccecf1e590886b6bcac08aaddfbf45 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 18 Dec 2024 11:40:22 +0000 Subject: [PATCH 02/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- app.py | 3 +-- website/flask_helpers.py | 2 -- website/html_validation.py | 5 ++++- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app.py b/app.py index daebea7e8ce..5f03761b94f 100644 --- a/app.py +++ b/app.py @@ -1,4 +1,5 @@ # coding=utf-8 +from flask import template_rendered import base64 import binascii import collections @@ -181,8 +182,6 @@ def load_all_adventures_for_index(customizations, subset=None): return all_adventures -from flask import template_rendered - def load_adventures_for_level(level, subset=None): """Load the adventures available to the current user at the given level. diff --git a/website/flask_helpers.py b/website/flask_helpers.py index 248f01a1453..fef27a20720 100644 --- a/website/flask_helpers.py +++ b/website/flask_helpers.py @@ -24,8 +24,6 @@ def render_template(filename, **kwargs): return rendered - - def proper_json_dumps(x, **kwargs): """Properly convert the input to JSON that deals with a bunch of edge cases. diff --git a/website/html_validation.py b/website/html_validation.py index 33a6d9c72c7..ad72db11e39 100644 --- a/website/html_validation.py +++ b/website/html_validation.py @@ -1,13 +1,16 @@ import re from collections import Counter + def validate_output_html(html): validate_no_duplicate_ids(html) ID_RE = re.compile('id="([^"]+)"') + def validate_no_duplicate_ids(html): ids = Counter(ID_RE.findall(html)) duplicates = [id for id, count in ids.items() if count > 1] - assert not duplicates, 'There are multiple HTML elements with the same id="..." on this page. IDs should be unique: %s' % ', '.join(duplicates) \ No newline at end of file + assert not duplicates, 'There are multiple HTML elements with the same id="..." on this page. IDs should be unique: %s' % ', '.join( + duplicates) From 9d37ddd09cd9829869140bf3329ef68879930e9a Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Wed, 18 Dec 2024 12:43:31 +0100 Subject: [PATCH 03/17] Linterrr --- app.py | 2 -- website/html_validation.py | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/app.py b/app.py index daebea7e8ce..8158d2790d2 100644 --- a/app.py +++ b/app.py @@ -181,8 +181,6 @@ def load_all_adventures_for_index(customizations, subset=None): return all_adventures -from flask import template_rendered - def load_adventures_for_level(level, subset=None): """Load the adventures available to the current user at the given level. diff --git a/website/html_validation.py b/website/html_validation.py index 33a6d9c72c7..be36136321b 100644 --- a/website/html_validation.py +++ b/website/html_validation.py @@ -10,4 +10,4 @@ def validate_output_html(html): def validate_no_duplicate_ids(html): ids = Counter(ID_RE.findall(html)) duplicates = [id for id, count in ids.items() if count > 1] - assert not duplicates, 'There are multiple HTML elements with the same id="..." on this page. IDs should be unique: %s' % ', '.join(duplicates) \ No newline at end of file + assert not duplicates, f'There are multiple HTML elements with the same id="..." on this page. IDs should be unique: {", ".join(duplicates)}' \ No newline at end of file From 963b4c35f5d19277b8247e1606e737d567d8e417 Mon Sep 17 00:00:00 2001 From: Rico Hermans Date: Thu, 19 Dec 2024 08:28:41 +0100 Subject: [PATCH 04/17] Shut up flake8 --- .flake8 | 2 ++ app.py | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 .flake8 diff --git a/.flake8 b/.flake8 new file mode 100644 index 00000000000..a6578a30e85 --- /dev/null +++ b/.flake8 @@ -0,0 +1,2 @@ +[flake8] +extend-ignore = E501 diff --git a/app.py b/app.py index 5f03761b94f..8158d2790d2 100644 --- a/app.py +++ b/app.py @@ -1,5 +1,4 @@ # coding=utf-8 -from flask import template_rendered import base64 import binascii import collections From b7ab1be0298c17825b83516d304871ce86ad7b82 Mon Sep 17 00:00:00 2001 From: Rico Hermans Date: Thu, 19 Dec 2024 09:00:14 +0100 Subject: [PATCH 05/17] Remove unused ids --- templates/admin/admin-adventures.html | 10 +++++----- templates/admin/admin-users.html | 20 +++++++++---------- templates/class-overview.html | 10 +++++----- .../partial-sortable-adventures.html | 18 ++++++++--------- .../customize-grid/partial-grid-tables.html | 12 +++++------ templates/hedy-page/editor-and-output.html | 10 +++++----- templates/incl/editor-and-output.html | 10 +++++----- templates/incl/parsons.html | 4 ++-- templates/printable/certificate.html | 4 ++-- templates/printable/cheatsheet.html | 6 +++--- templates/public-adventures/body.html | 16 +++++++-------- templates/quiz/partial-review-quiz.html | 2 +- templates/super-teacher/tags.html | 2 +- 13 files changed, 62 insertions(+), 62 deletions(-) diff --git a/templates/admin/admin-adventures.html b/templates/admin/admin-adventures.html index 1d9b16ddefb..33482b07b5c 100644 --- a/templates/admin/admin-adventures.html +++ b/templates/admin/admin-adventures.html @@ -10,12 +10,12 @@

Total amount of shown adventures: {{ adventures|length } - + - - - - + + + + diff --git a/templates/admin/admin-users.html b/templates/admin/admin-users.html index 6d04d08d54e..c830ce63ab8 100644 --- a/templates/admin/admin-users.html +++ b/templates/admin/admin-users.html @@ -9,7 +9,7 @@
Update the tags of
-
+
{{_('certified_teacher')}} ⊙
@@ -133,20 +133,20 @@

Total amount of shown users: {{ users|length }}

- + - + - + - + - - - + + + - + @@ -170,7 +170,7 @@

Total amount of shown users: {{ users|length }}

hx-post="/admin/mark-as-teacher/{{user.username}}"> diff --git a/templates/class-overview.html b/templates/class-overview.html index c38b9af7fbc..09dae0ef9bc 100644 --- a/templates/class-overview.html +++ b/templates/class-overview.html @@ -29,7 +29,7 @@

{{ _('teachers')}}

- {% if not is_second_teacher and class_info.teacher == username %}{% endif %} + {% if not is_second_teacher and class_info.teacher == username %}{% endif %} {% for second_teacher in class_info.second_teachers %} - + {% if second_teacher.username != username %} @@ -70,9 +70,9 @@

{{ _('teachers')}}

{% endif %} {% endif %} {% endfor %} - + {% endfor %} {% if not class_info.students %} @@ -64,16 +64,16 @@

-
Adventure nameAdventure name CreatorLevelLast updatedPublicViewLevelLast updatedPublicView
Username Email Last loginBirth yearBirth year LanguageKeyword langKeyword lang CountryTagsTags Teacher Super Teacher
- {{_('username')}} {{_('role')}} {{_('programs')}}{{_('remove')}}{{_('remove')}}
{{second_teacher.username}}{% if second_teacher.username == class_info.teacher %}{{_('creator').lower()}}{% else %}{{second_teacher.role}}{% endif %}{% if second_teacher.username == class_info.teacher %}{{_('creator').lower()}}{% else %}{{second_teacher.role}}{% endif %} - {% if second_teacher.username != class_info.teacher %}{{_('select_adventures')}} {{_('reset_button')}}
-
{{_('updating_indicator')}}
-
-
+
{% for adventure in adventures[level|int] %} -
{{ adventure.long_name }} {# Hidden input so we can send the adventures names in the request to the server #} -
+
{% endfor %}
@@ -91,4 +91,4 @@

{{_('select_adventures')}}

}) } - \ No newline at end of file + \ No newline at end of file diff --git a/templates/customize-grid/partial-grid-tables.html b/templates/customize-grid/partial-grid-tables.html index 07411a3e43b..583f857857b 100644 --- a/templates/customize-grid/partial-grid-tables.html +++ b/templates/customize-grid/partial-grid-tables.html @@ -36,7 +36,7 @@
+
- + @@ -82,8 +82,8 @@

- - + + - + From f087a5b5d5bef78653935baa050897006947bd40 Mon Sep 17 00:00:00 2001 From: Rico Hermans Date: Thu, 19 Dec 2024 09:07:25 +0100 Subject: [PATCH 06/17] MOar --- templates/admin/admin-adventures.html | 2 +- templates/admin/admin-users.html | 4 ++-- templates/customize-grid/partial-grid-tables.html | 4 ++-- templates/htmx-classes-table.html | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/templates/admin/admin-adventures.html b/templates/admin/admin-adventures.html index 33482b07b5c..b68466c85fe 100644 --- a/templates/admin/admin-adventures.html +++ b/templates/admin/admin-adventures.html @@ -11,7 +11,7 @@

Total amount of shown adventures: {{ adventures|length }

- + diff --git a/templates/admin/admin-users.html b/templates/admin/admin-users.html index c830ce63ab8..97509a1af98 100644 --- a/templates/admin/admin-users.html +++ b/templates/admin/admin-users.html @@ -144,8 +144,8 @@

Total amount of shown users: {{ users|length }}

- - + + diff --git a/templates/customize-grid/partial-grid-tables.html b/templates/customize-grid/partial-grid-tables.html index 583f857857b..9ccad6f094c 100644 --- a/templates/customize-grid/partial-grid-tables.html +++ b/templates/customize-grid/partial-grid-tables.html @@ -89,11 +89,11 @@

- diff --git a/templates/htmx-classes-table.html b/templates/htmx-classes-table.html index 4b79930e712..0ced797e177 100644 --- a/templates/htmx-classes-table.html +++ b/templates/htmx-classes-table.html @@ -26,7 +26,7 @@ - - {% if is_admin %} - + {% endif %} {% else %} {% if user["username"] != adventure["creator"] %} @@ -83,17 +83,17 @@ {% else %} + class="green-btn"> {{_('edit_adventure')}} {% endif %} - {% if is_super_teacher %} - {{_('last_update')}}: {{ class.date|format_date }}

--> - -
{% if is_program %} {% if is_admin %} - + {% endif %} {% else %} {% if user["username"] != adventure["creator"] %} @@ -93,7 +93,7 @@ hx-swap="none" _="on htmx:afterRequest if detail.xhr.status == 200 then window.location.reload()"> {% if is_super_teacher %} - Date: Thu, 19 Dec 2024 12:52:15 +0100 Subject: [PATCH 12/17] Oops --- templates/htmx-adventure-card.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/htmx-adventure-card.html b/templates/htmx-adventure-card.html index 69f8e5c9ed5..84d7b5044fc 100644 --- a/templates/htmx-adventure-card.html +++ b/templates/htmx-adventure-card.html @@ -60,7 +60,7 @@ 
{{ adventure.code }}
{%endif%}
-
{% if is_program %} From 623aa488b057a8ccaacbe6aa064525beb6fe2a33 Mon Sep 17 00:00:00 2001 From: Rico Hermans Date: Thu, 19 Dec 2024 13:59:54 +0100 Subject: [PATCH 13/17] Remove id="warningbox_icon" --- templates/custom-elements-templates.html | 2 +- templates/customize-adventure.html | 22 +++++++++++----------- templates/embedded-editor.html | 4 ++-- templates/hedy-page/editor-and-output.html | 2 +- templates/incl/editor-and-output.html | 2 +- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/templates/custom-elements-templates.html b/templates/custom-elements-templates.html index 43bede6d1a7..587441b6f0a 100644 --- a/templates/custom-elements-templates.html +++ b/templates/custom-elements-templates.html @@ -30,7 +30,7 @@

-
+
{{_('customize_adventure')}}: {{adventure.name}}

-
+
@@ -130,23 +130,23 @@

{{_('customize_adventure')}}: {{adventure.name}}

class="appearance-none w-full bg-gray-200 border border-gray-200 text-gray-700 py-3 px-4 ltr:pr-8 rtl:pl-8 rounded" hx-trigger="keyup[keyCode == 13]" data-autosaved="true" - hx-post="/tags/create/{{ adventure.id }}" + hx-post="/tags/create/{{ adventure.id }}" hx-target="#tags_list" hx-swap="outerHTML" _="on click event.stopPropagation() on keypress if event.keyCode == 13 event.stopPropagation() end on htmx:beforeRequest event.preventDefault() then if my value == '' then detail.xhr.abort() end - on htmx:afterRequest set my value to '' then focus()" + on htmx:afterRequest set my value to '' then focus()" > - +
{{_('customize_adventure')}}: {{adventure.name}}
- {{ render_partial('htmx-tags-list.html', tags=adventure_tags, adventure_id=adventure.id, creator=adventure.creator) }} + {{ render_partial('htmx-tags-list.html', tags=adventure_tags, adventure_id=adventure.id, creator=adventure.creator) }}
@@ -168,7 +168,7 @@

{{_('customize_adventure')}}: {{adventure.name}}

-
- +
@@ -197,7 +197,7 @@

{{_('adventure_exp_classes')}}

{% endif %}
- +
{{_('students')}} {{_('class_logs')}} {{_('highest_level_reached')}}{{_('adventures_ticked')}}{{_('adventures_ticked')}} {{_('actions')}}
{{ student }} {{ students_info[student].last_login }} {{ students_info[student].highest_level }} {{ students_info[student].adventures_ticked }} {{ students_info[student].highest_level }} {{ students_info[student].adventures_ticked }}
diff --git a/templates/incl/editor-and-output.html b/templates/incl/editor-and-output.html index 8bf44f2a2f6..1eb0b551a89 100644 --- a/templates/incl/editor-and-output.html +++ b/templates/incl/editor-and-output.html @@ -10,7 +10,7 @@
-
+

An error occurred.

@@ -91,9 +91,9 @@
- +
-
+
    {% if not raw %} {% endfor %}
{% endif %} -
+

{{_('what_should_my_code_do')}}

diff --git a/templates/printable/certificate.html b/templates/printable/certificate.html index b33cbed55fb..d462ead03b4 100644 --- a/templates/printable/certificate.html +++ b/templates/printable/certificate.html @@ -22,13 +22,13 @@

{{_('fun_statistics_ms

{{_('highest_level_reached')}} ✅: {{quiz_level}}

{{_('highest_quiz_score')}} ✅: {{quiz_score}}

{{_('number_programs')}} 💻: {{user_program_count}}

-

{{_('longest_program')}} 👩‍💻: {{longest_program}}

+

{{_('longest_program')}} 👩‍💻: {{longest_program}}

- + diff --git a/templates/public-adventures/body.html b/templates/public-adventures/body.html index 3ed08ed5a3b..99a5a838754 100644 --- a/templates/public-adventures/body.html +++ b/templates/public-adventures/body.html @@ -11,7 +11,7 @@ name="search" value="{{currentSearch}}" {% if currentSearch %}autofocus{% endif %} - hx-trigger="input changed delay:500ms, search" + hx-trigger="input changed delay:500ms, search" hx-post="/public-adventures/filter?level={{level}}&lang={{selectedLang}}&tag={{selectedTag}}&search={{currentSearch}}" hx-target="#public_adventures" placeholder="{{_('search')}}" @@ -29,7 +29,7 @@ >{{ level }} {% endfor %} - +
- +
{% for tag in available_tags %} {% set selected = tag in selectedTag %} - {% endfor %} @@ -63,8 +63,8 @@ {% if false and is_super_teacher %}
- {% if get_certificate %}
NamePopularityPopularity Delete
Adventure nameCreatorCreator Level Last updated Public TagsTeacherSuper TeacherTeacherSuper Teacher