Bug 18419 - Broken patron-blank image in viewlog.tt
Summary: Broken patron-blank image in viewlog.tt
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Target Milestone: ---
Assignee: Owen Leonard
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-11 19:28 UTC by Eric Phetteplace
Modified: 2017-12-07 22:20 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
screenshot (96.66 KB, image/png)
2017-04-11 19:28 UTC, Eric Phetteplace
Details
Bug 18419 - Broken patron-blank image in viewlog.tt (6.63 KB, patch)
2017-04-25 16:26 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 18419 - Broken patron-blank image in viewlog.tt (6.70 KB, patch)
2017-04-26 13:17 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18419 - Broken patron-blank image in viewlog.tt (7.06 KB, patch)
2017-04-26 19:36 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Phetteplace 2017-04-11 19:28:21 UTC
Created attachment 62062 [details]
screenshot

The theme's patron-blank.png image is incorrectly referenced from the viewlog.tt template, resulting in a broken image. One can see this by going to the logs tool, selecting the "Patrons" module, and filling in a borrowernumber under "object" which matches a patron who does not have a custom image. Screenshot attached.

I believe this is due to the interaction between line #13 of circ-menu.inc and lines #58-66 of viewlog.tt. The circ menu include expects a value like "/intranet-tmpl" for the "interface" variable (this is what's used on circulation.tt, for instance) while viewlog.tt has a switch statement that assigns the value "Intranet" to the variable, causing the link to break.

I'd offer a patch except I'm not sure how the interface value is used throughout viewlog.tt and its includes, or why it appears to refer to the same interface (the intranet) in two different forms for two different templates.
Comment 1 Owen Leonard 2017-04-25 16:26:56 UTC
Created attachment 62691 [details] [review]
Bug 18419 - Broken patron-blank image in viewlog.tt

Because of a variable name collision the blank patron image doesn't
display in viewlog.tt. This patch moves the image from the template to
the stylesheet to avoid this problem.

This patch also replaces the blank patron image PNG file with an SVG
file. SVG support is wide enough to begin using whereever possible.

This patch also removes some inline CSS from circ-menu.inc and puts it
in the global stylesheet.

To test, apply the patch and clear your browser cache if necessary.

- Enable the patronimages system preference.
- View a patron account which lacks a patron image. All views (details,
  fines, notices, etc) should show the "blank" patron image, including
  the modification log view.
- View a patron account which has a patron image and check that it still
  displays correctly in all views.
Comment 2 Josef Moravec 2017-04-26 13:17:38 UTC
Created attachment 62729 [details] [review]
[SIGNED-OFF] Bug 18419 - Broken patron-blank image in viewlog.tt

Because of a variable name collision the blank patron image doesn't
display in viewlog.tt. This patch moves the image from the template to
the stylesheet to avoid this problem.

This patch also replaces the blank patron image PNG file with an SVG
file. SVG support is wide enough to begin using whereever possible.

This patch also removes some inline CSS from circ-menu.inc and puts it
in the global stylesheet.

To test, apply the patch and clear your browser cache if necessary.

- Enable the patronimages system preference.
- View a patron account which lacks a patron image. All views (details,
  fines, notices, etc) should show the "blank" patron image, including
  the modification log view.
- View a patron account which has a patron image and check that it still
  displays correctly in all views.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 3 Jonathan Druart 2017-04-26 19:36:30 UTC
Created attachment 62746 [details] [review]
Bug 18419 - Broken patron-blank image in viewlog.tt

Because of a variable name collision the blank patron image doesn't
display in viewlog.tt. This patch moves the image from the template to
the stylesheet to avoid this problem.

This patch also replaces the blank patron image PNG file with an SVG
file. SVG support is wide enough to begin using whereever possible.

This patch also removes some inline CSS from circ-menu.inc and puts it
in the global stylesheet.

To test, apply the patch and clear your browser cache if necessary.

- Enable the patronimages system preference.
- View a patron account which lacks a patron image. All views (details,
  fines, notices, etc) should show the "blank" patron image, including
  the modification log view.
- View a patron account which has a patron image and check that it still
  displays correctly in all views.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 4 Jonathan Druart 2017-04-26 19:38:00 UTC
I promise I will try to not remove this empty div, but I cannot guarantee it :)

I would have replaced the li id with something more explicit that "patronbasics".
Comment 5 Owen Leonard 2017-04-27 13:00:50 UTC
(In reply to Jonathan Druart from comment #4)
> I promise I will try to not remove this empty div, but I cannot guarantee it
> :)

I felt it was better to have the empty div as the placeholder for the blank image so that there was a more specific target for CSS (more specific than the <li>).
Comment 6 Kyle M Hall 2017-04-28 10:56:34 UTC
Pushed to master for 17.05, thanks Owen!
Comment 7 Katrin Fischer 2017-05-13 12:15:37 UTC
This patch has been pushed to 16.11.x and will be in 16.11.08.
Comment 8 Eric Phetteplace 2017-05-15 03:26:33 UTC
Such a quick fix! Thanks everyone!
Comment 9 Mason James 2017-05-24 02:09:16 UTC
Pushed to 16.05.x, for 16.05.13 release