Bug 40816

Summary: Upgrade DataTables from 2.1.8 to >2.3.4
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Staff interfaceAssignee: Jonathan Druart <jonathan.druart>
Status: Signed Off --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: blawlor, david, dcook, gmcharlt, oleonard, paul.derscheid, victor
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41137
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37024
GIT URL: Initiative type: ---
Sponsorship status: --- Comma delimited list of Sponsors:
Crowdfunding goal: 0 Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 36640    
Bug Blocks: 38259    
Attachments: Bug 40816: DT - Update css and js files
Bug 40816: Cherry-pick previous manual changes after update
Bug 40816: [DO NOT PUSH] Use .css instead of .min.css
Bug 40816: Fix sort icons
Bug 40816: Remove !important from the min version
Bug 40816: DT - Update css and js files
Bug 40816: Cherry-pick previous manual changes after update
Bug 40816: Fix sort icons
Bug 40816: Remove !important from the min version
Bug 40816: DT - Update css and js files - OPAC
Bug 40816: Remove !important from css files - OPAC
Bug 40816: Adjust about page
Bug 40816: (follow-up) CSS updates for table sorting icons
Bug 40816: (follow-up) CSS updates for table sorting icons
Bug 40816: (follow-up) Standardize sort icon alignment
Bug 40816: Fix a failing Selenium test
Bug 40816: Fix Auth/csrf.ts
Bug 40816: Ignore 'ResizeObserver loop limit exceeded'

Description Jonathan Druart 2025-09-16 08:15:18 UTC
eg. bug 38259 needs DataTables 2.3+ for lengthLabels
Comment 1 Jonathan Druart 2025-09-16 08:19:26 UTC
Created attachment 186448 [details] [review]
Bug 40816: DT - Update css and js files
Comment 2 Jonathan Druart 2025-09-16 08:19:34 UTC
Created attachment 186449 [details] [review]
Bug 40816: Cherry-pick previous manual changes after update

This is a cherry-pick of
  commit e0193051e173557351d3da6697cb5f9c3e801185
  Bug 36640: (follow-up) CSS fixes for DataTables controls

I have only kept the changes to datatables.css
Comment 3 Jonathan Druart 2025-09-16 08:19:40 UTC
Created attachment 186450 [details] [review]
Bug 40816: [DO NOT PUSH] Use .css instead of .min.css

What the clean way to generate the min version?
Comment 4 Jonathan Druart 2025-09-16 08:19:48 UTC
Created attachment 186451 [details] [review]
Bug 40816: Fix sort icons
Comment 5 Jonathan Druart 2025-09-16 08:21:02 UTC
Why do we have a lot of duplicated CSS rules between _tables.scss and _common.scss?

Shouldn't all the .dataTable rules from _common be moved to _tables?
Comment 6 Jonathan Druart 2025-11-03 15:52:33 UTC
Created attachment 188979 [details] [review]
Bug 40816: Remove !important from the min version
Comment 7 Jonathan Druart 2025-11-03 15:53:11 UTC
(In reply to Jonathan Druart from comment #5)
> Why do we have a lot of duplicated CSS rules between _tables.scss and
> _common.scss?
> 
> Shouldn't all the .dataTable rules from _common be moved to _tables?

Let's do that on a separate bug report if needed.
Comment 8 Owen Leonard 2025-11-03 19:27:51 UTC
Created attachment 188991 [details] [review]
Bug 40816: DT - Update css and js files

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 9 Owen Leonard 2025-11-03 19:27:54 UTC
Created attachment 188992 [details] [review]
Bug 40816: Cherry-pick previous manual changes after update

This is a cherry-pick of
  commit e0193051e173557351d3da6697cb5f9c3e801185
  Bug 36640: (follow-up) CSS fixes for DataTables controls

I have only kept the changes to datatables.css

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 10 Owen Leonard 2025-11-03 19:27:57 UTC
Created attachment 188993 [details] [review]
Bug 40816: Fix sort icons

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 11 Owen Leonard 2025-11-03 19:28:00 UTC
Created attachment 188994 [details] [review]
Bug 40816: Remove !important from the min version

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 12 Owen Leonard 2025-11-04 13:26:27 UTC
We should do the same for the OPAC shouldn't we?
Comment 13 Jonathan Druart 2025-11-04 13:40:36 UTC
(In reply to Owen Leonard from comment #12)
> We should do the same for the OPAC shouldn't we?

Yes!
Comment 14 David Nind 2025-11-06 17:38:24 UTC
The license details in the about page also needs updating.
Comment 15 Jonathan Druart 2025-11-14 13:15:26 UTC
Created attachment 189599 [details] [review]
Bug 40816: DT - Update css and js files - OPAC

Patch from commit 372bfb4
Comment 16 Jonathan Druart 2025-11-14 13:15:28 UTC
Created attachment 189600 [details] [review]
Bug 40816: Remove !important from css files - OPAC

Patch from commit 20bdf8f
Comment 17 Jonathan Druart 2025-11-14 13:15:29 UTC
Created attachment 189601 [details] [review]
Bug 40816: Adjust about page

Patch from commit 42a5269
Comment 18 Jonathan Druart 2025-11-14 13:20:06 UTC
It's still missing a "Fix sort icons" patch for the OPAC.
Comment 19 Owen Leonard 2025-11-14 13:50:51 UTC
Created attachment 189605 [details] [review]
Bug 40816: (follow-up) CSS updates for table sorting icons
Comment 20 Jonathan Druart 2025-11-14 14:00:04 UTC
Created attachment 189606 [details] [review]
Bug 40816: (follow-up) CSS updates for table sorting icons

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 21 Jonathan Druart 2025-11-14 14:00:32 UTC
OPAC is ready for testing! :)

Thanks Owen!
Comment 22 Victor Grousset/tuxayo 2025-12-17 06:58:27 UTC
This test fails with the patches:
t/db_dependent/selenium/patrons_search.t

#   at t/db_dependent/selenium/patrons_search.t line 660.
Error while executing command: no such element: Unable to locate element: //table[@id="memberresultst"]//th[@aria-label="Date of birth: Activate to sort"]/input at /usr/share/perl5/Selenium/Remote/Driver.pm line 411.


(I'm sorting out Cypress issues also check them)
Comment 23 Jonathan Druart 2025-12-18 09:19:32 UTC
Ha, I think it's where I left this bug and why I was asking comment 5.

There is the sorting arrows not correctly displayed in the headers, and the class is no longer on the the but in a span child.

https://snipboard.io/olhaYc.jpg
Comment 24 Owen Leonard 2025-12-18 11:58:22 UTC
Created attachment 190614 [details] [review]
Bug 40816: (follow-up) Standardize sort icon alignment

DataTables wants to change the alignment of the sorting icons when it
detects the column contents as numeric or date. This patch overrides
that.
Comment 25 Jonathan Druart 2025-12-18 12:22:54 UTC
Created attachment 190615 [details] [review]
Bug 40816: Fix a failing Selenium test

Patch from commit 3040a99
Comment 26 Victor Grousset/tuxayo 2025-12-18 19:51:28 UTC
Thanks for the fix :)


(In reply to Victor Grousset/tuxayo from comment #22)
> (I'm sorting out Cypress issues also check them)

Finally!

Unfortunately there are 3 tests that fails with the patches:

cypress run --config video=false --spec "t/cypress/integration/Tools/ManageMarcImport_spec.ts,t/cypress/integration/Auth/csrf.ts,t/cypress/integration/ERM/Agreements_spec.ts"

-------------
  1) loads the manage MARC import page
       upload a MARC record:
     AssertionError: Timed out retrying after 10000ms: Expected to find content: 'View batch' but never did.
      at Context.eval (webpack://koha/./t/cypress/integration/Tools/ManageMarcImport_spec.ts:75:11)
----------------
  1) CSRF
       Add:
     AssertionError: Timed out retrying after 10000ms: Expected to find element: `select[name='libraries_length']`, but never found it.
      at Context.eval (webpack://koha/./t/cypress/integration/Auth/csrf.ts:112:11)

--------------------
  1) Agreement CRUD operations
       Delete agreement:

     Error: The following error originated from your application code, not from Cypress.

  > ResizeObserver loop limit exceeded

When Cypress detects uncaught errors originating from your application it will automatically fail the current test.

This behavior is configurable, and you can choose to turn this off by listening to the `uncaught:exception` event.

https://on.cypress.io/uncaught-exception-from-application
    at <unknown> (http://koha:8081/cgi-bin/koha/erm/agreements:0:0)
Comment 27 Victor Grousset/tuxayo 2025-12-18 20:13:48 UTC
QA script complains about koha-tmpl/opac-tmpl/lib/datatables/datatables.min.js but it shouldn't be checked so it's a false positive. Since it's only checked when being touched by a commit, that doesn't seem to an issue even if left as is.
Comment 28 Jonathan Druart 2026-01-06 13:58:16 UTC
Created attachment 190937 [details] [review]
Bug 40816: Fix Auth/csrf.ts

Patch from commit b69cdec
Comment 29 Jonathan Druart 2026-01-06 13:58:18 UTC
Created attachment 190938 [details] [review]
Bug 40816: Ignore 'ResizeObserver loop limit exceeded'

I have NO idea how to fix this error. I don't see it on the Koha UI,
neither using Cypress UI.

It seems safe for now to ignore this error (The internet says so).
Comment 30 Jonathan Druart 2026-01-06 13:58:33 UTC
(In reply to Victor Grousset/tuxayo from comment #26)
> cypress run --config video=false --spec
> "t/cypress/integration/Tools/ManageMarcImport_spec.ts,t/cypress/integration/
> Auth/csrf.ts,t/cypress/integration/ERM/Agreements_spec.ts"
> 
> -------------
>   1) loads the manage MARC import page
>        upload a MARC record:
>      AssertionError: Timed out retrying after 10000ms: Expected to find
> content: 'View batch' but never did.
>       at Context.eval
> (webpack://koha/./t/cypress/integration/Tools/ManageMarcImport_spec.ts:75:11)

Known random failure, see bug 41368.


>   1) CSRF
>        Add:
>      AssertionError: Timed out retrying after 10000ms: Expected to find
> element: `select[name='libraries_length']`, but never found it.
>       at Context.eval
> (webpack://koha/./t/cypress/integration/Auth/csrf.ts:112:11)

Should be fixed. 

>   1) Agreement CRUD operations
>        Delete agreement:
> 
>      Error: The following error originated from your application code, not
> from Cypress.
> 
>   > ResizeObserver loop limit exceeded
> 
> When Cypress detects uncaught errors originating from your application it
> will automatically fail the current test.
> 
> This behavior is configurable, and you can choose to turn this off by
> listening to the `uncaught:exception` event.
> 
> https://on.cypress.io/uncaught-exception-from-application
>     at <unknown> (http://koha:8081/cgi-bin/koha/erm/agreements:0:0)

I am stuck on this one, I've decided to ignore the error for now.