fix: rendering tables [BB-6729]

[Cup0fCoffee]

Description

This PR fixes issues related to tables generated by TinyMCE editor.

Tables are not rendered correctly in LMS - certain element, inline attributes and styles are not rendered, when JavaScript is not allowed for the XBlock. Enabling JavaScript fixes the issue, because it disables the sanitization of the rendered HTML. We still want the HTML to be sanitized, so we add the relevant elements, attributes and styles to the allow list for sanitization.

Screenshots

LMS

Before

After

Studio preview

Before

After

Testing instructions

1. Setup devstack (for development Maple was used, but Nutmeg or master should also work)
2. Clone xblock-html repo into <devstack-root>/src/ and change the branch
3. Start lms, studio, and learning MFE:

    make lms-up studio-up frontend-app-learning-up

4. Install xblock-html into lms:

   make lms-shell lms-restart
   pip install -e /edx/src/xblock-html/ && exit

5. Install xblock-html into studio:

   make studio-shell studio-restart
   pip install -e /edx/src/xblock-html/ && exit

6. Login into studio as staff (e.g. edx@example.com)
7. Go to demo course advanced settings, and add html5 and excluded_html5 in Advanced Module List
8. Go to demo course outline, and create a new section, subsection and unit
9. In the new unit add html5 xblock - Advanced > Text
10. Create a new table using the visual editor (or insert html provided below)
11. Save and publish
12. Check that the issues described in the description are fixed

The following HTML was generated using TinyMCE visual editor using all the settings which affect the tables styling:

<table style="border-collapse: collapse; width: 100%; height: 34px; background-color: #c2e0f4; border-color: blue; border-style: solid; margin-left: auto; margin-right: auto;" cellspacing="1" cellpadding="1" border="10"><caption></caption>
<thead>
<tr style="height: 10px; background-color: #2dc26b; border-color: green; border-style: solid; text-align: center;">
<td style="width: 33.1779%; height: 10px;" scope="col"></td>
<td style="width: 33.1779%; height: 10px;" scope="col"></td>
<td style="width: 33.1779%; height: 10px;" scope="col"></td>
</tr>
</thead>
<tbody>
<tr style="height: 14px;">
<td style="width: 33.1779%; height: 14px;"></td>
<td style="width: 33.1779%; height: 14px; background-color: #3598db; text-align: center; vertical-align: middle; border: 2px solid #236fa1;"></td>
<td style="width: 33.1779%; height: 14px;"></td>
</tr>
</tbody>
<tfoot>
<tr style="height: 10px; background-color: #b96ad9; border-color: purple; border-style: solid; text-align: center;">
<td style="width: 33.1779%; height: 10px;"></td>
<td style="width: 33.1779%; height: 10px;"></td>
<td style="width: 33.1779%; height: 10px;"></td>
</tr>
</tfoot>
</table>

Notes

I added a unit test for checking how html of table generated by TinyMCE would be rendered. I had to modify the html by sorting the inline attributes and styles in alphabetical order, so the it matches the sanitizer's output. The test has been useful during the development, but can be difficult to maintain, so can be removed from the PR.

[Agrendalath]

Please remember to bump the version.

[Cup0fCoffee]

@Agrendalath I see CI is failing, and I know why, but this might be intentional. It has to do with the bleach version in the requirements/base.txt. There seems to be a reason to why it's unpinned - version 4 is used in Nutmeg and Maple and version 5 is used in Olive. So if it was pinned to either version, it would overwrite the version which is installed by edx-platform.
The issue is that, because the version is unpinned, the latest one is installed, which is 5.1. But for it to work, it also requires bleach[css] to be installed, which is commented out in the requirements file.

The simplest way I thought of fixing this for tests is to add the following to requirements/test.txt:

bleach>=5.0.0
bleach[css]>=5.0.0

This would fix the tests, but wouldn't fix the quality check, because it would still complain about passing unexpected arguments in the fallback for version 4.1.0.

[Agrendalath]

@Cup0fCoffee,

There seems to be a reason to why it's unpinned - version 4 is used in Nutmeg and Maple and version 5 is used in Olive. So if it was pinned to either version, it would overwrite the version which is installed by edx-platform.

It won't overwrite the installed version. This XBlock does not have lots of dependencies on its own, so only the most important ones are defined in setup.py - it doesn't load anything from the requirements directory. If the version was specified in setup.py, it would indeed overwrite the installed package.

I just pinned bleaching to the version we're using (a35641d).

[Agrendalath]

👍

• I tested this: checked that relatively complex tables are rendered the same way in LMS, Studio, and the TinyMCE editor
• I read through the code
• I checked for accessibility issues: n/a
• Includes documentation: n/a
• I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository: n/a

[Cup0fCoffee]

@Agrendalath

It won't overwrite the installed version. This XBlock does not have lots of dependencies on its own, so only the most important ones are defined in setup.py - it doesn't load anything from the requirements directory. If the version was specified in setup.py, it would indeed overwrite the installed package.

Oh, I made an assumption that requirements/ are used to install the dependencies.

Can we pin a 5.* version, instead of 4.*, since the code was already made compatible with it, and we will migrate to it eventually anyways?

[Agrendalath]

@Cup0fCoffee,

Can we pin a 5.* version, instead of 4.*, since the code was already made compatible with it, and we will migrate to it eventually anyways?

Looks like we've backported bleach version update to Nutmeg, so it makes sense to test its compatibility. We still need to support Maple, though, so I've added CI tests for both versions.