Bug 10442

Summary: Remove references to non-standard "error" class
Product: Koha Reporter: Owen Leonard <oleonard>
Component: TemplatesAssignee: Owen Leonard <oleonard>
Status: CLOSED FIXED QA Contact: Katrin Fischer <katrin.fischer>
Severity: normal    
Priority: P5 - low CC: fridolin.somers, gmcharlt, katrin.fischer, kyle, martin.renvoize, nick, srdjan, wizzyrea
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 10442 - Remove references to non-standard "error" class
Bug 10442 - Remove references to non-standard "error" class
[PASSED QA] Bug 10442 - Remove references to non-standard "error" class
Bug 10442: Remove references to non-standard "error" class
Bug 10442: Remove references to non-standard "error" class
Bug 10442: Remove references to non-standard "error" class
Bug 10442: Remove references to non-standard "error" class
Bug 10442: Remove references to non-standard "error" class
Bug 10442: Remove references to non-standard "error" class

Description Owen Leonard 2013-06-10 14:14:25 UTC
There are a few references in the templates to a very old "error" class which should not be used anymore. Generally speaking '<div class="dialog alert">' should be used instead.
Comment 1 Owen Leonard 2013-06-10 14:44:59 UTC Comment hidden (obsolete)
Comment 2 Srdjan Jankovic 2013-06-11 02:59:54 UTC
I'm not sure that replacing & with &amp; in href was in order
Comment 3 Kyle M Hall 2013-06-14 12:00:12 UTC
Owen, can you please address Srjdan's comment 2?
Comment 4 Owen Leonard 2013-06-25 13:11:23 UTC
(In reply to Srdjan Jankovic from comment #2)
> I'm not sure that replacing & with &amp; in href was in order

While not strictly within the scope of the bug, unescaped ampersands are a violation of coding guidelines (invalid HTML) and should be corrected.
Comment 5 Liz Rea 2013-06-29 06:16:04 UTC Comment hidden (obsolete)
Comment 6 Katrin Fischer 2013-06-30 10:01:04 UTC Comment hidden (obsolete)
Comment 7 Galen Charlton 2013-07-01 01:56:24 UTC
Comment on attachment 19296 [details] [review]
[PASSED QA] Bug 10442 - Remove references to non-standard "error" class

Review of attachment 19296 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt
@@ -171,5 @@
>      [% END %]
>  </h2>
>  
> -<div class="error" style="display:none"></div>
> -

I'm not sure whether simply removing this is OK.  There is code in additem.js that looks for the existence of a div whose CSS class is "error"; if such a div is not present, an error message (e.g., when trying to add a duplicate item) would have no place to be displayed.

Setting bug back to "Signed-off" for QA to look into this.  If I'm right, additem.js will need to be modified, e.g., to use a div ID rather than a div class to locate the placeholder for an error message.

It may be necessary to apply the patch for bug 10483 to be able to test a situation where attempting to add an item during the process of creating a new order results in an error.
Comment 8 Srdjan Jankovic 2013-07-10 01:03:16 UTC
> While not strictly within the scope of the bug, unescaped ampersands are a
> violation of coding guidelines (invalid HTML) and should be corrected.

Even in href? I'm not sure about that.
Comment 9 Owen Leonard 2018-03-13 14:50:47 UTC Comment hidden (obsolete)
Comment 10 Katrin Fischer 2018-07-08 16:41:07 UTC
Patch doesn't apply, can you please rebase?
Comment 11 Owen Leonard 2018-07-16 18:48:32 UTC Comment hidden (obsolete)
Comment 12 Owen Leonard 2018-08-13 13:51:36 UTC Comment hidden (obsolete)
Comment 13 Owen Leonard 2018-09-04 16:10:04 UTC Comment hidden (obsolete)
Comment 14 Andrew Isherwood 2018-10-30 11:33:41 UTC Comment hidden (obsolete)
Comment 15 Katrin Fischer 2018-10-31 06:59:07 UTC
Created attachment 81646 [details] [review]
Bug 10442: Remove references to non-standard "error" class

The CSS for <div class="error"> is obsolete and should not be used. This
patch removes the definition from the main CSS file and corrects
instances of its use in the templates to the standard <div class="dialog
alert">.

To test:

- In Acquisitions -> Late orders, locate an order from a vendor which
  doesn't have an email address. Selecting that order and clicking
  "Claim" should trigger an error dialog, "This vendor has no email." It
  should be styled correctly.

- With  AcqCreateItem set to "when placing an order," add to an existing
  order using the "From a new (empty) record" option. Add two items with
  identical barcodes and submit the form. A error should show at
  the top of the page.

- With AcqCreateItem set to "when receiving an order," receive an order
  and add two items with identical barcodes. Submitting the form should
  trigger an error message at the top of the page.

With the remaining cases I don't know how to trigger the errors in
question, so a visual check of the changes may be required:

- Administration -> Funds -> "You are not authorized to modify this
  fund"
- Administration -> Search engine configuration
  (/admin/searchengine/elasticsearch/mappgings.pl) -> Various
  modification errors.
- With the  AutoEmailOpacUser preference set to "send," adding a patron
  without an email address can trigger an error, "This member has no
  email."
- With plugins enabled, and installed, there are error messages
  displayed under various circumstances.

Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 16 Nick Clemens 2018-10-31 15:02:20 UTC
Awesome work all!

Pushed to master for 18.11
Comment 17 Martin Renvoize 2018-11-09 15:53:57 UTC
Pushed to 18.05.x for 18.05.06
Comment 18 Fridolin Somers 2018-11-28 13:33:37 UTC
Pushed to 17.11.x for 17.11.13