Bug 15921 - DataTables JavaScript files included twice on many pages
Summary: DataTables JavaScript files included twice on many pages
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-02-25 19:29 UTC by Owen Leonard
Modified: 2016-12-05 21:23 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 15921: Do not include datatables js/css files twice (1.41 KB, patch)
2016-02-29 08:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15921: Do not include datatables js/css files twice (1.47 KB, patch)
2016-03-14 02:58 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 15921: Do not include datatables js/css files twice (2.23 KB, patch)
2016-03-21 14:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15921: Do not include datatables js/css files twice (2.45 KB, patch)
2016-03-23 09:30 UTC, Marc Véron
Details | Diff | Splinter Review
[PASSED QA] Bug 15921: Do not include datatables js/css files twice (2.51 KB, patch)
2016-03-23 22:24 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2016-02-25 19:29:30 UTC
datatables.inc loads jquery.dataTables.min.js, dataTables.buttons.min.js, and buttons.colVis.min.js

columns_settings.inc also loads dataTables.buttons.min.js, buttons.colVis.min.js

This would be fine if each were used independently, but columns_settings.inc is always included along with datatables.inc.
Comment 1 Owen Leonard 2016-02-25 19:34:45 UTC
Is there any reason to include dataTables.buttons.min.js and buttons.colVis.min.js on pages which do not have columns configuration enabled?
Comment 2 Jonathan Druart 2016-02-29 08:52:01 UTC
datatables.inc loads dataTables.buttons.min.js and buttons.colVis.min.js since bug 15285: Update common files because the dom param now contains 'B'. The DT init fails with it does not know what 'B' means.

So yes, now it does not make sense to have them included in both files.
Comment 3 Jonathan Druart 2016-02-29 08:57:13 UTC Comment hidden (obsolete)
Comment 4 Srdjan Jankovic 2016-03-14 02:58:19 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2016-03-20 22:55:46 UTC
With this patch "columns visibility" on administration/currencies doesn't appear to work anymore.
Comment 6 Jonathan Druart 2016-03-21 14:16:53 UTC Comment hidden (obsolete)
Comment 7 Marc Véron 2016-03-23 09:30:54 UTC Comment hidden (obsolete)
Comment 8 Katrin Fischer 2016-03-23 22:24:50 UTC
Created attachment 49481 [details] [review]
[PASSED QA] Bug 15921: Do not include datatables js/css files twice

datatables.inc loads dataTables.buttons.min.js and buttons.colVis.min.js
since bug 15285: Update common files because the dom param now contains
'B'. The DT init fails with it does not know what 'B' means.

Test plan:
Test tables using buttons (columns visibility), they should work as
before this patch.

Tested with patron search and administration/currencies, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 9 Brendan Gallagher 2016-03-24 15:50:47 UTC
Pushed to Master - should be in the May 2016 release.  Thanks!
Comment 10 Julian Maurice 2016-04-07 05:52:13 UTC
Not needed in stable versions, status changed to RESOLVED