Bug 22957 - Remove type attribute from script tags: Staff client includes 1/2
Summary: Remove type attribute from script tags: Staff client includes 1/2
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Owen Leonard
QA Contact: Testopia
URL:
Keywords: Academy
Depends on:
Blocks: 22956
  Show dependency treegraph
 
Reported: 2019-05-21 18:22 UTC by Owen Leonard
Modified: 2019-08-06 16:21 UTC (History)
4 users (show)

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


Attachments
Bug 22957: Remove type attribute from script tags: Staff client includes 1/2 (7.49 KB, patch)
2019-05-21 18:55 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 22957: Remove type attribute from script tags: Staff client includes 1/2 (7.55 KB, patch)
2019-07-05 17:42 UTC, Nadine Pierre
Details | Diff | Splinter Review
Bug 22957: Remove type attribute from script tags: Staff client includes 1/2 (7.65 KB, patch)
2019-07-06 06:42 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 2019-05-21 18:22:32 UTC
This patch will remove the "type" attribute from <script> tags in the following staff client include files:

 - intranet-tmpl\prog\en\includes\acquisitions-toolbar.inc
 - intranet-tmpl\prog\en\includes\additem.js.inc
 - intranet-tmpl\prog\en\includes\browser-strings.inc
 - intranet-tmpl\prog\en\includes\catalog-strings.inc
 - intranet-tmpl\prog\en\includes\datatables.inc
 - intranet-tmpl\prog\en\includes\format_price.inc
 - intranet-tmpl\prog\en\includes\greybox.inc
Comment 1 Owen Leonard 2019-05-21 18:55:25 UTC
Created attachment 89950 [details] [review]
Bug 22957: Remove type attribute from script tags: Staff client includes 1/2

This patch removes the "type" attribute from <script> tags in several
staff client include files. Also removed: Obsolete "//<![CDATA[ //]]>"
markers.

This patch also makes minor indentation changes, so diff using the "-w"
flag.

To test, apply the patch and confirm that examples of affected pages
work properly without any JavaScript errors in the browser console:

 - Acquisitions -> Vendor (uses acuisitions-toolbar.inc)
 - Acquisitions -> Vendor -> Add to basket -> From a new (empty) record
   (uses additem.js.inc)
 - Catalog -> Search results -> Bibliographic detail view. (uses
   browser-strings.inc, catalog-strings.inc, datatables.inc, and
   format_price.inc )
 - Tools -> Label creator -> Manage -> Label batches -> Export batch
   (uses greybox.inc)

Validating the HTML source of any of these pages should return no errors
related to the "type" attribute.
Comment 2 M. Tompsett 2019-07-04 19:09:56 UTC
Quick question about & vs &amp; and HTML vs XML validation with respect to the removal of CDATA in acquisitions-toolbar.inc?
Are we ever validating as XML or XHTML? If not, okay. If so, then why remove CDATA?
Comment 3 Owen Leonard 2019-07-05 11:52:59 UTC
(In reply to M. Tompsett from comment #2)

> Are we ever validating as XML or XHTML? If not, okay. If so, then why remove
> CDATA?

As of version 3.12 Koha uses an HTML5 doctype: https://wiki.koha-community.org/wiki/Coding_Guidelines#HTML_Templates. HTML5 documents are not required to be well-formed XML.
Comment 4 Nadine Pierre 2019-07-05 17:42:26 UTC
Created attachment 91337 [details] [review]
Bug 22957: Remove type attribute from script tags: Staff client includes 1/2

This patch removes the "type" attribute from <script> tags in several
staff client include files. Also removed: Obsolete "//<![CDATA[ //]]>"
markers.

This patch also makes minor indentation changes, so diff using the "-w"
flag.

To test, apply the patch and confirm that examples of affected pages
work properly without any JavaScript errors in the browser console:

 - Acquisitions -> Vendor (uses acuisitions-toolbar.inc)
 - Acquisitions -> Vendor -> Add to basket -> From a new (empty) record
   (uses additem.js.inc)
 - Catalog -> Search results -> Bibliographic detail view. (uses
   browser-strings.inc, catalog-strings.inc, datatables.inc, and
   format_price.inc )
 - Tools -> Label creator -> Manage -> Label batches -> Export batch
   (uses greybox.inc)

Validating the HTML source of any of these pages should return no errors
related to the "type" attribute.

Signed-off-by: Nadine Pierre <nadine.pierre@inLibro.com>
Comment 5 Katrin Fischer 2019-07-06 06:42:30 UTC
Created attachment 91382 [details] [review]
Bug 22957: Remove type attribute from script tags: Staff client includes 1/2

This patch removes the "type" attribute from <script> tags in several
staff client include files. Also removed: Obsolete "//<![CDATA[ //]]>"
markers.

This patch also makes minor indentation changes, so diff using the "-w"
flag.

To test, apply the patch and confirm that examples of affected pages
work properly without any JavaScript errors in the browser console:

 - Acquisitions -> Vendor (uses acuisitions-toolbar.inc)
 - Acquisitions -> Vendor -> Add to basket -> From a new (empty) record
   (uses additem.js.inc)
 - Catalog -> Search results -> Bibliographic detail view. (uses
   browser-strings.inc, catalog-strings.inc, datatables.inc, and
   format_price.inc )
 - Tools -> Label creator -> Manage -> Label batches -> Export batch
   (uses greybox.inc)

Validating the HTML source of any of these pages should return no errors
related to the "type" attribute.

Signed-off-by: Nadine Pierre <nadine.pierre@inLibro.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 6 Martin Renvoize 2019-07-15 11:21:53 UTC
Nice work!

Pushed to master for 19.11.00
Comment 7 Fridolin SOMERS 2019-07-30 05:40:54 UTC
Pushed to 19.05.x for 19.05.03
Comment 8 Lucas Gass 2019-08-06 16:21:12 UTC
backported to 18.11.x for 18.11.09