From 3719a163ca079ca341207646971ede463814cd00 Mon Sep 17 00:00:00 2001 From: Maxim Beder Date: Thu, 22 Sep 2022 18:41:23 +0200 Subject: [PATCH 1/9] test: add test for rendering tables Added a test for testing that tables are rendered correctly. I.e. without html elements, attributes or styles removed by the sanitizer. --- tests/test_basics.py | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/test_basics.py b/tests/test_basics.py index f2b5961..d908995 100644 --- a/tests/test_basics.py +++ b/tests/test_basics.py @@ -11,6 +11,32 @@ import html_xblock +TABLE_HTML = """ + + + + + + + + + + + + + + + + + + + + + + +
+""" # noqa: E501 + class TestHTMLXBlock(unittest.TestCase): """ @@ -43,6 +69,16 @@ def test_render_with_unsafe(self): fragment.content ) + def test_render_table_without_allow_js(self): + """ + Test that tables are rendered correctly without allowing js. + """ + field_data = DictFieldData({'data': TABLE_HTML}) + block = html_xblock.HTML5XBlock(self.runtime, field_data, None) + self.assertEqual(block.allow_javascript, False) + fragment = block.student_view() + self.assertIn(TABLE_HTML, fragment.content) + def test_render_allow_js(self): """ Test a basic rendering with javascript enabled. From c1228e4cc7cd87e4460a1b8e0bca6d96b3ca0161 Mon Sep 17 00:00:00 2001 From: Maxim Beder Date: Thu, 22 Sep 2022 18:48:34 +0200 Subject: [PATCH 2/9] fix: fix table rendering in LMS Tables weren't being rendered correctly in LMS, because the sanitizer would filter out html element, attributes and styles of the tables. Added html elements, attributes and styles to the list of allow lists to fix the rendering. --- html_xblock/bleaching.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/html_xblock/bleaching.py b/html_xblock/bleaching.py index e6aacf0..2620834 100644 --- a/html_xblock/bleaching.py +++ b/html_xblock/bleaching.py @@ -91,8 +91,11 @@ def _get_allowed_tags(self): 'ul', 'p', 'pre', + 'caption', 'table', + 'thead', 'tbody', + 'tfoot', 'th', 'tr', 'td' @@ -118,7 +121,9 @@ def _get_allowed_attributes(self): 'pre': ['class'], 'span': ['style'], 'ul': [], - 'table': ['class'], + 'table': ['class', 'style', 'border', 'cellspacing', 'cellpadding'], + 'tr': ['style'], + 'td': ['style', 'scope'], } if not self.strict: @@ -136,7 +141,11 @@ def _get_allowed_styles(self): :return: Allowed styles depending on the bleaching mode """ - styles = ['font-family', 'text-align', 'color', 'text-decoration', 'padding-left', 'padding-right'] + styles = [ + 'background-color', 'border', 'border-collapse', 'border-color', 'border-style', 'color', 'font-family', + 'height', 'margin-left', 'margin-right', 'padding-left', 'padding-right', 'text-align', 'text-decoration', + 'vertical-align', 'width', + ] if not self.strict: styles += ['list-style-type', 'font-size', 'border-width', 'margin'] From 1b2df6f25c5e6fe6a5a93e7f7dd735ae6d3e898b Mon Sep 17 00:00:00 2001 From: Maxim Beder Date: Thu, 22 Sep 2022 23:22:07 +0200 Subject: [PATCH 3/9] refactor: emphasise which styles and scripts are for editing in studio Renamed functions and files for styles and scripts which are loaded only when editing the XBlock in studio, to make it easier to distinguish from the files which are loaded in the LMS view or in studio preview. --- html_xblock/html.py | 16 ++++++++-------- .../static/css/{html.css => html_edit.css} | 0 html_xblock/static/js/{html.js => html_edit.js} | 0 3 files changed, 8 insertions(+), 8 deletions(-) rename html_xblock/static/css/{html.css => html_edit.css} (100%) rename html_xblock/static/js/{html.js => html_edit.js} (100%) diff --git a/html_xblock/html.py b/html_xblock/html.py index 5b6a3ec..7a10916 100644 --- a/html_xblock/html.py +++ b/html_xblock/html.py @@ -97,8 +97,8 @@ def studio_view(self, context=None): # pylint: disable=unused-argument frag.content = xblock_loader.render_django_template('static/html/studio.html', context) - self.add_stylesheets(frag) - self.add_scripts(frag) + self.add_edit_stylesheets(frag) + self.add_edit_scripts(frag) js_data = { 'editor': self.editor, @@ -152,24 +152,24 @@ def workbench_scenarios(): """), ] - def add_stylesheets(self, frag): + def add_edit_stylesheets(self, frag): """ - A helper method to add all necessary styles to the fragment. + A helper method to add all styles to the fragment necesesary for edit. :param frag: The fragment that will hold the scripts. """ - frag.add_css(self.resource_string('static/css/html.css')) + frag.add_css(self.resource_string('static/css/html_edit.css')) if self.editor == 'raw': frag.add_css_url(settings.STATIC_URL + 'js/vendor/CodeMirror/codemirror.css') - def add_scripts(self, frag): + def add_edit_scripts(self, frag): """ - A helper method to add all necessary scripts to the fragment. + A helper method to add all scripts to the fragment necessary for edit. :param frag: The fragment that will hold the scripts. """ frag.add_javascript_url(settings.STATIC_URL + 'js/vendor/tinymce/js/tinymce/tinymce.full.min.js') frag.add_javascript_url(settings.STATIC_URL + 'js/vendor/tinymce/js/tinymce/themes/silver/theme.min.js') - frag.add_javascript(self.resource_string('static/js/html.js')) + frag.add_javascript(self.resource_string('static/js/html_edit.js')) frag.add_javascript(loader.load_unicode('public/studio_edit.js')) if self.editor == 'raw': diff --git a/html_xblock/static/css/html.css b/html_xblock/static/css/html_edit.css similarity index 100% rename from html_xblock/static/css/html.css rename to html_xblock/static/css/html_edit.css diff --git a/html_xblock/static/js/html.js b/html_xblock/static/js/html_edit.js similarity index 100% rename from html_xblock/static/js/html.js rename to html_xblock/static/js/html_edit.js From b48ea1c31095426c070b2b07aa9a0f6a8b0d5675 Mon Sep 17 00:00:00 2001 From: Maxim Beder Date: Thu, 22 Sep 2022 23:26:23 +0200 Subject: [PATCH 4/9] fix: fix for tables' styles in studio preview of html xblock In studio preview of the xblocks there are global styles that remove borders from elements, including tables and elements that are parts of the tables. These global styles would overwrite the browser default styles for table borders, especially when `border` attribute was set on the table. The fix provides styles that are loaded only in the studio preview. The new styles try to fix the issue by overwriting the global styles and setting the border to the same styles as they would be by default. --- html_xblock/html.py | 3 + html_xblock/static/css/html_preview.css | 77 +++++++++++++++++++++++++ 2 files changed, 80 insertions(+) create mode 100644 html_xblock/static/css/html_preview.css diff --git a/html_xblock/html.py b/html_xblock/html.py index 7a10916..e081bf2 100644 --- a/html_xblock/html.py +++ b/html_xblock/html.py @@ -80,6 +80,9 @@ def student_view(self, context=None): # pylint: disable=unused-argument frag.add_css(self.resource_string('public/plugins/codesample/css/prism.css')) frag.add_javascript(self.resource_string('public/plugins/codesample/js/prism.js')) + if getattr(self.runtime, 'is_author_mode', False): + frag.add_css(self.resource_string('static/css/html_preview.css')) + return frag def studio_view(self, context=None): # pylint: disable=unused-argument diff --git a/html_xblock/static/css/html_preview.css b/html_xblock/static/css/html_preview.css new file mode 100644 index 0000000..da09460 --- /dev/null +++ b/html_xblock/static/css/html_preview.css @@ -0,0 +1,77 @@ +/* This css file contains styles that should be used for the XBlock preview in + * studio. + * + * It contains the fix for rendering tables in studio preview, which is + * caused by global studio styles. These global styles set `border: 0;` for + * tables and their elements, which removes the borders from these elements. */ + +/* fall back to default browser styles for all rows; the inline styles (like + * border style and color) would overwrite it */ +.studio-xblock-wrapper [data-block-type="html5"] table > * > tr { + border: unset; +} + +/* fall back to default browser styles, if border attribute is not set; the + * inline styles (like border style and color) would overwrite it */ +.studio-xblock-wrapper [data-block-type="html5"] table:not([border]) { + border: unset; +} + +/* default styles for any table that had border attribute set; the inline + * styles (like border style and color) would overwrite it */ +.studio-xblock-wrapper [data-block-type="html5"] table[border] { + border: 1px solid black; +} + +/* change the border width, depending on the explicit value of the border + * attribute */ +.studio-xblock-wrapper [data-block-type="html5"] table[border="1"] { + border-width: 1px; +} + +.studio-xblock-wrapper [data-block-type="html5"] table[border="2"] { + border-width: 2px; +} + +.studio-xblock-wrapper [data-block-type="html5"] table[border="3"] { + border-width: 3px; +} + +.studio-xblock-wrapper [data-block-type="html5"] table[border="4"] { + border-width: 4px; +} + +.studio-xblock-wrapper [data-block-type="html5"] table[border="5"] { + border-width: 5px; +} + +.studio-xblock-wrapper [data-block-type="html5"] table[border="6"] { + border-width: 6px; +} + +.studio-xblock-wrapper [data-block-type="html5"] table[border="7"] { + border-width: 7px; +} + +.studio-xblock-wrapper [data-block-type="html5"] table[border="8"] { + border-width: 8px; +} + +.studio-xblock-wrapper [data-block-type="html5"] table[border="9"] { + border-width: 9px; +} + +.studio-xblock-wrapper [data-block-type="html5"] table[border="10"] { + border-width: 10px; +} +/* etc until the value we think is reasonable */ + +.studio-xblock-wrapper [data-block-type="html5"] table[border] > * > tr > td, +.studio-xblock-wrapper [data-block-type="html5"] table[border] > * > tr > th, +.studio-xblock-wrapper [data-block-type="html5"] table[border] > * > td, +.studio-xblock-wrapper [data-block-type="html5"] table[border] > * > th, +.studio-xblock-wrapper [data-block-type="html5"] table[border] > td, +.studio-xblock-wrapper [data-block-type="html5"] table[border] > th { + border-width: thin; + border-style: inset; +} From 314bd3458be6ec272e3d04c29ea989ea6acb9e8d Mon Sep 17 00:00:00 2001 From: Maxim Beder Date: Tue, 27 Sep 2022 09:49:33 +0200 Subject: [PATCH 5/9] chore: bump up the version --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index b3e451b..5098cb0 100644 --- a/setup.py +++ b/setup.py @@ -23,7 +23,7 @@ def package_data(pkg, roots): setup( name='html-xblock', - version='1.2.0', + version='1.2.1', description='HTML XBlock will help creating and using a secure and easy-to-use HTML blocks', license='AGPL v3', packages=[ From 70c4ed025ca047b42f8c039c33b6613cb3d72999 Mon Sep 17 00:00:00 2001 From: Maxim Beder Date: Tue, 27 Sep 2022 09:51:23 +0200 Subject: [PATCH 6/9] chore: remove commented out styles --- html_xblock/static/css/html_edit.css | 4 ---- 1 file changed, 4 deletions(-) diff --git a/html_xblock/static/css/html_edit.css b/html_xblock/static/css/html_edit.css index d371b28..02a3ce1 100644 --- a/html_xblock/static/css/html_edit.css +++ b/html_xblock/static/css/html_edit.css @@ -1,7 +1,3 @@ -/* .modal-type-html5 [class*="view-"] .modal-lg.modal-editor .modal-content { - height: 500px; -} */ - .html-editor, .wrapper-comp-settings.is-active.editor-with-buttons { height: 320px; From 1f7afd9b572adcea3442f855af0b051ba8ef4e66 Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Wed, 28 Sep 2022 18:54:24 +0200 Subject: [PATCH 7/9] test: fix bleaching --- html_xblock/bleaching.py | 2 +- requirements/base.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/html_xblock/bleaching.py b/html_xblock/bleaching.py index 2620834..ccaa0bc 100644 --- a/html_xblock/bleaching.py +++ b/html_xblock/bleaching.py @@ -42,6 +42,7 @@ def get_cleaner(self): considering safe in the platform. """ if CSSSanitizer: + # pylint: disable=unexpected-keyword-arg cleaner = bleach.Cleaner( tags=self._get_allowed_tags(), attributes=self._get_allowed_attributes(), @@ -54,7 +55,6 @@ def get_cleaner(self): # used in Maple and Nutmeg release of edx-platform. This can be removed # for Olive release which uses bleach 5.0.0 - # pylint: disable-next=unexpected-keyword-arg cleaner = bleach.Cleaner( tags=self._get_allowed_tags(), attributes=self._get_allowed_attributes(), diff --git a/requirements/base.txt b/requirements/base.txt index 40c858e..f17fef6 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -4,6 +4,6 @@ git+https://github.com/edx/xblock-utils.git@2.1.1#egg=xblock-utils==2.1.1 django-statici18n==1.9.0 edx-i18n-tools==0.5.3 Mako==1.1.3 -bleach # version unpinned to make it compatible with Maple/Nutmeg (4.1.0) +bleach==4.1.0 # version pinned for Maple/Nutmeg # bleach[css]==5.0.0 # Use this for Olive django~=2.2 From 47fb2e44b0c5d2b7ec718cffbae43fc82919056f Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Wed, 28 Sep 2022 18:56:03 +0200 Subject: [PATCH 8/9] build: update CircleCI image --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index b7316d6..73d7c45 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1,7 +1,7 @@ defaults: &DEFAULT working_directory: /home/circleci/xblock-html docker: - - image: circleci/python:3.8 + - image: cimg/python:3.8 version: 2 From ee500837b4f7cf3181ed2bc1ac25b2ff3f65f8cc Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Fri, 30 Sep 2022 12:37:51 +0200 Subject: [PATCH 9/9] build: update test requirements, add tests for Nutmeg --- .circleci/config.yml | 42 +++++++++++++++++++++++++++++++++++++++- Makefile | 3 +-- html_xblock/bleaching.py | 12 ++++++------ pylintrc | 3 ++- requirements/base.txt | 12 +++++------- requirements/quality.txt | 12 +++++------- requirements/test.txt | 15 ++++---------- 7 files changed, 64 insertions(+), 35 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 73d7c45..a080382 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -11,7 +11,7 @@ jobs: steps: - checkout - restore_cache: - key: deps1-{{ .Branch }}--{{ checksum "requirements/base.txt" }}-{{ checksum "requirements/test.txt" }}-{{ checksum "requirements/quality.txt" }} + key: deps2-{{ .Branch }}--{{ checksum "requirements/base.txt" }}-{{ checksum "requirements/test.txt" }}-{{ checksum "requirements/quality.txt" }} - run: name: Installing requirements command: | @@ -41,6 +41,22 @@ jobs: name: Running tests command: make unit-coverage + test_nutmeg: + <<: *DEFAULT + steps: + - checkout + - attach_workspace: + at: /tmp/workspace + - run: + name: Activate virtualenv + command: echo 'source /tmp/workspace/env/bin/activate' >> $BASH_ENV + - run: + name: Install bleach 5 + command: pip install bleach[css]==5.0.0 + - run: + name: Running tests + command: make unit-coverage + quality: <<: *DEFAULT steps: @@ -54,6 +70,22 @@ jobs: name: Test Quality command: make quality + quality_nutmeg: + <<: *DEFAULT + steps: + - checkout + - attach_workspace: + at: /tmp/workspace + - run: + name: Activate virtualenv + command: echo 'source /tmp/workspace/env/bin/activate' >> $BASH_ENV + - run: + name: Install bleach 5 + command: pip install bleach[css]==5.0.0 + - run: + name: Test Quality + command: make quality + workflows: version: 2 build-test: @@ -65,3 +97,11 @@ workflows: - quality: requires: - build + - test_nutmeg: + requires: + - test + - quality + - quality_nutmeg: + requires: + - test + - quality diff --git a/Makefile b/Makefile index 33c79ca..9b5082c 100644 --- a/Makefile +++ b/Makefile @@ -33,7 +33,6 @@ test_requirements: base_requirements ## Install requirements needed by test envi pip install -q -r requirements/test.txt --exists-action w requirements: base_requirements test_requirements ## Installs all requirements needed by developmenent and test environments - pip install -e . @echo "Finished installing requirements." quality: ## Run quality tests and checks @@ -48,7 +47,7 @@ unit-coverage: clean ## Run coverage and unit tests coverage run ./manage.py test coverage html coverage xml - diff-cover coverage.xml --html-report diff-cover.html + diff-cover coverage.xml --compare-branch origin/master --html-report diff-cover.html unit: clean ## Run unit tests mkdir var/ diff --git a/html_xblock/bleaching.py b/html_xblock/bleaching.py index ccaa0bc..520e04a 100644 --- a/html_xblock/bleaching.py +++ b/html_xblock/bleaching.py @@ -9,8 +9,8 @@ # NOTE: # The bleach library changes the way CSS Styles are cleaned in # version 5.0.0. Since the edx-platform uses version 4.1.0 in - # Maple and Nutmeg, this import is handled within a try block. - # This try block CAN BE REMOVED after Olive + # Maple, this import is handled within a try block. + # This try block CAN BE REMOVED for Nutmeg CSSSanitizer = None @@ -42,7 +42,7 @@ def get_cleaner(self): considering safe in the platform. """ if CSSSanitizer: - # pylint: disable=unexpected-keyword-arg + # pylint: disable-next=unexpected-keyword-arg cleaner = bleach.Cleaner( tags=self._get_allowed_tags(), attributes=self._get_allowed_attributes(), @@ -52,9 +52,9 @@ def get_cleaner(self): ) else: # NOTE: This is maintaining backward compatibility with bleach 4.1.0 - # used in Maple and Nutmeg release of edx-platform. This can be removed - # for Olive release which uses bleach 5.0.0 - + # used in Maple release of edx-platform. This can be removed + # for Nutmeg release which uses bleach 5.0.0 + # pylint: disable-next=unexpected-keyword-arg cleaner = bleach.Cleaner( tags=self._get_allowed_tags(), attributes=self._get_allowed_attributes(), diff --git a/pylintrc b/pylintrc index aeeedea..a5ec76b 100644 --- a/pylintrc +++ b/pylintrc @@ -36,5 +36,6 @@ min-similarity-lines=8 ignore=D101,D400,D401,D200,D203,D205,D212,D215,D404,D405,D406,D407,D408,D409,D410,D411,D412,D413,D414 [pycodestyle] -exclude = .git,migrations max-line-length=120 + +# TODO: Use `edx_lint write pylintrc` to generate rules. diff --git a/requirements/base.txt b/requirements/base.txt index f17fef6..9f64e72 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -1,9 +1,7 @@ # Requirements for app run -git+https://github.com/edx/xblock-utils.git@2.1.1#egg=xblock-utils==2.1.1 -django-statici18n==1.9.0 -edx-i18n-tools==0.5.3 -Mako==1.1.3 -bleach==4.1.0 # version pinned for Maple/Nutmeg -# bleach[css]==5.0.0 # Use this for Olive -django~=2.2 +xblock-utils==2.2.0 +edx-i18n-tools==0.9.1 +bleach==4.1.0 # version pinned for Maple +# bleach[css]==5.0.0 # Use this for Nutmeg +django==3.2.13 diff --git a/requirements/quality.txt b/requirements/quality.txt index b50d128..47aa11b 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -1,9 +1,7 @@ # Requirements for code quality checks -isort==4.3.21 -astroid==2.3.3 -pycodestyle==2.6.0 -pydocstyle==5.0.2 -pylint==2.4.2 -caniusepython3==7.2.0 -edx-lint==1.4.1 +isort==5.10.1 +pycodestyle==2.9.1 +pydocstyle==6.1.1 +pylint==2.15.3 +edx-lint==5.3.0 diff --git a/requirements/test.txt b/requirements/test.txt index 41de6d2..dfb80a3 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -1,14 +1,7 @@ # Requirements for test runs -codecov==2.0.15 -diff-cover==2.6.1 -django-nose==1.4.4 -lazy==1.4 -pytest-cov==2.8.1 -tox==3.15.0 -tox-battery==0.6.0 -mock==3.0.5 - -# Github requirements -git+https://github.com/edx/django-pyfs.git@2.1#egg=django-pyfs==2.1 +codecov==2.1.12 +diff-cover==7.0.1 +pytest-cov==4.0.0 +mock==4.0.3 xblock-sdk