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: Main
Hardware: All All
: P5 - low normal
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)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:

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
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

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