Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The Following dataTable Example Fails #9763

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

bozzit
Copy link

@bozzit bozzit commented May 15, 2024

What does this pull request (PR) do? / Que fait cette demande « pull » (PR)?

Client of Mine PSPC, tried to upgrade to a version of wet-boew > 4.0.39.1. Which implements, DOMPurify, and interferes with jquery.datatable.js

https://datatables.net/examples/api/row_details.html

In the example the row.child(format(row.data())).show(); line of code inserts dom structure "", that gets Stripped out by DOMPurify and causes a javascript error in the console.

Uncaught TypeError: $(...).addClass(...).html(...)[0] is undefined in Firefox

There is already an dataTableAllowedTag I just added the structure to it, and that ensures this row.child dataTable function doesn't get interfered with.

Additional information (optional) / Information additionnelle (optionel)

General checklist / Liste de contrôle générale
Make your own list for the purpose of your Pull request. /// Faites votre propre liste en fonction des besoins de votre demande « pull ».

  • Create/update documentation /// Créer/mettre à jour la documentation
  • [x ] Build and test PR's code /// Rouler le script de compilation et tester le code de la PR
  • Validate changes against WCAG for accessibility /// Valider les changements avec WCAG pour l'accessibilité
  • Ensure documentation is bilingual /// S'assurer que la documentation soit bilingue

Steve Bourgeois and others added 2 commits May 15, 2024 09:16
…nto a datatable failes as of DOMPurify

https://datatables.net/examples/api/row_details.html
 row.child(format(row.data())).show();
inserts dom structure "<tr><td></td></tr>", that gets Stripped out by DOMPurify
There is already an dataTableAllowedTag I just added the structure to it
@Garneauma
Copy link
Contributor

Pre-approved assuming the proposed fix passes testing and security.

@Garneauma Garneauma self-requested a review May 21, 2024 14:04
@Garneauma
Copy link
Contributor

@bozzit I have tested locally and the proposed solution does not seem to work. I have tested the following:

  • Create:
    <table id="TestTable" class="table"><thead><tr><th>1</th></tr></thead><tbody><tr><td>Test</td></tr></tbody</table>
  • In JS tried the following:
    $( "#TestTable tbody" ).append( "<tr><td>test 2</td></tr>" );
    $( "#TestTable tbody" ).html( "<tr><td>test 2</td></tr>" );

Both tests have the HTML stripped out.

Could you provide a working example of your fix in this pull request? Maybe inside /plugins/tables/tables-en-hbs.

Thank you.

@Garneauma Garneauma added the Need: Test example Describe a test with step by step, code sample, screen capture and the expected results. label May 27, 2024
@bozzit
Copy link
Author

bozzit commented May 27, 2024

$( "#TestTable tbody" ).append( "<tr><td></td></tr>" );

This is exactly what datable Inserts on child inserts, hence why I added "<tr><td></td></tr>" to the Datable exception dataTableAllowedTag = [

@Garneauma
Copy link
Contributor

$( "#TestTable tbody" ).append( "<tr><td></td></tr>" );

This is exactly what datable Inserts on child inserts, hence why I added "<tr><td></td></tr>" to the Datable exception dataTableAllowedTag = [

@bozzit Yes, but have you tested your fix to make sure it works? When I try your fix in my local environment, it doesn't seem to work...

@bozzit
Copy link
Author

bozzit commented May 27, 2024

Well I modified our wet-boew.js file that way here at PSPC and after doing so the following example I setup as a testcase now works.
https://datatables.net/examples/api/row_details.html

@bozzit
Copy link
Author

bozzit commented May 27, 2024

feel free to reach out via MS Teams if you are on GC

@Garneauma
Copy link
Contributor

@bozzit Would you mind joining our office hours meeting tomorrow so we can go through this together? I think it would be easier if you could show us your page. You can either join between 1-2 PM EST or we can book a time slot anywhere between 2-4 PM EST. Let me know your preference. Here is the link to the meeting: Office hours

@bozzit
Copy link
Author

bozzit commented May 27, 2024

Sure I can hop on between 1-2pm Tomorrow 28th May

@Garneauma
Copy link
Contributor

Garneauma commented May 28, 2024

Hi @bozzit, after further review of the DataTables Child rows example, it is very likely that the generated code is not WCAG compliant. Unfortunately, the added cell has content that is not associated to any table header, which is like it's floating in the table, which fails WCAG info and relationships criterion.

It is possible to manually define the header associated to the newly created cell, but this would need to be added to the many modifications we made to DataTables inside WET-BOEW'S Tables plugin to make it WCAG compliant.

This means that this fix would most likely not be able to be implemented.

If you still want to go ahead with this fix, we will need a working example done in tables-en.html and tables-fr.html. We will then be able to go through an accessibility assessment of this new feature and go from there.

Thank you.

@duboisp
Copy link
Member

duboisp commented May 28, 2024

To complement @Garneauma comment,

There is 2 major issue where:

Note that it is possible to address and adapt the datatable in a way to avoid those issue and provide that user experience while being compliant to WCAG. In order to make it compliant you will need to apply a post-transformation to the datatable generated markup when such expand/collapse in row feature is used. Such post-transformation to the markup can applied by leveraging the jQuery event of the datatable and other events. Like how we do it here: https://github.com/wet-boew/wet-boew/blob/master/src/plugins/tables/tables.js#L236

See the following code sample snippet, which is not compliant, that I took from the working example on Datatable.net

<tbody>
	<tr class="dt-hasChild">
		<td class="dt-type-numeric dt-control"></td>
		<td class="sorting_1">Airi Satou</td>
		<td>Accountant</td>
		<td>Tokyo</td>
		<td class="dt-type-numeric">$162,700</td>
	</tr>
	<tr data-dt-row="4">
		<td colspan="5">
			<dl>
				<dt>Full name:</dt>
				<dd>Airi Satou</dd>
				<dt>Extension number:</dt>
				<dd>5407</dd>
				<dt>Extra info:</dt>
				<dd>And any further details here (images etc)...</dd>
			</dl>
		</td>
	</tr>
	<tr>
		<td class="dt-type-numeric dt-control"></td>
		<td class="sorting_1">Angelica Ramos</td>
		<td>Chief Executive Officer (CEO)</td>
		<td>London</td>
		<td class="dt-type-numeric">$1,200,000</td>
	</tr>
	
	...

</tbody>

image

Next step will be for you to build that use case in a wet-boew working example, then figure out the code transformation required to ensure WCAG compliance and then figuring out how to hook on the datatable to make that transformation and then implement it into wet-boew.

@duboisp duboisp added Query: Feature Request Feature: Table enhancement (datatable) Need: Reusable prototype Need a developper that would create a reusable and configurable WET plugin prototype labels May 28, 2024
@bozzit
Copy link
Author

bozzit commented May 29, 2024

Yes I understand the WCAG issues but this is not within the scope of the initial problem...

Regardless of what is displayed in that row and how the on click action / button / <td> or even on click event from a button that might be external to the table, the issue is that row.child causes a jquery.datatable.js to fail with a nasty JS error and breaks wet-boew other functionality.

Uncaught TypeError: P(...).add

There are other workarounds to insert a row in a DataTable i know, but IMHO code that is suggested by the makers of DataTable on how to implement functionality should never break wet-boew. This error should be caught and dealt with, logged or a graceful message instead of failing.

That said let me know if you really need a full working testcase that passes all WCAG standard, as like I said initially in this comment, I believe that those issues are not relevant. We have full QC / Accessibility testing done on all our pages, and those issues would be addressed by that process, and the Web/Datatable Programmer would be mad to fix those issues before pages make it to our production environment.

@Garneauma
Copy link
Contributor

@bozzit I understand your frustration. However, we only support the features that are described/shown in our working examples in WET-BOEW/GCWeb. We do not support every third-party feature as it would be way too much effort to through all of them and make sure they are WCAG compliant and compatible with WET-BOEW/GCWeb.

That said, you have two options:

  1. Create a working example of your use case and make sure it is WCAG 2.1 AA compliant.
  2. Load your own version of jQuery and DataTables on your page and handle the entire plugin's WCAG compliancy on your own.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature: Table enhancement (datatable) Need: Reusable prototype Need a developper that would create a reusable and configurable WET plugin prototype Need: Test example Describe a test with step by step, code sample, screen capture and the expected results. Query: Feature Request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants