Bug 16575 - Irregular behaviour using window.print() followed by window.location.href=
Summary: Irregular behaviour using window.print() followed by window.location.href=
Status: Pushed to Stable
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Owen Leonard
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-05-24 07:00 UTC by David Cook
Modified: 2018-08-02 13:38 UTC (History)
6 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
Bug 16575 - Irregular behaviour using window.print() followed by window.location.href= (3.16 KB, patch)
2016-05-30 02:27 UTC, David Cook
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16575 - Irregular behaviour using window.print() followed by window.location.href= (3.21 KB, patch)
2016-05-30 04:43 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Bug 16575: Irregular behaviour using window.print() followed by window.location.href (12.28 KB, patch)
2018-05-30 14:20 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 16575: (follow-up) Compiled CSS (7.28 KB, patch)
2018-05-30 14:20 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 16575: Irregular behaviour using window.print() followed by window.location.href (12.29 KB, patch)
2018-07-08 16:11 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 16575: (follow-up) Compiled CSS (86.59 KB, patch)
2018-07-08 16:11 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 16575: (follow-up) Compiled CSS (7.26 KB, patch)
2018-07-15 18:01 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 16575: Irregular behaviour using window.print() followed by window.location.href (12.39 KB, patch)
2018-07-16 08:10 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16575: (follow-up) Compiled CSS (7.35 KB, patch)
2018-07-16 08:10 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2016-05-24 07:00:27 UTC
As we noticed in bug 11014, different browsers handle "window.print()" differently.

In the case of window.print() followed by window.location.href=, I've found in IE 11 on Windows 8.1 that the browser changes the window location before the system print dialogue even opens. If I try to mitigate that using the timer from bug 11014, it works in IE and Chrome, but it breaks in Firefox. If I try to use the onafterprint in IE and Firefox, it works in IE but it breaks in Firefox.

I started wondering if we even need to be using this window.print() followed by window.location.href= and it occurred to me that we really don't. We should just be using print.css.  

Patch to follow soon...
Comment 1 David Cook 2016-05-30 02:27:10 UTC Comment hidden (obsolete)
Comment 2 David Cook 2016-05-30 02:42:41 UTC
At some point, it would be prudent to strip the now unnecessary "print_basket" stuff out of opac-basket.tt. I didn't include it here, as it wasn't necessary for the actual bug fix.

I also didn't want to do the work of removing all that code, if QA decides that this fix is no good, so I'm happy to add a follow-up once QA is happy with this patch.
Comment 3 Srdjan Jankovic 2016-05-30 04:43:49 UTC Comment hidden (obsolete)
Comment 4 Srdjan Jankovic 2016-05-30 04:46:05 UTC
I think you should have included js removal with the patch, not used so should go.
What's the story with the intranet basket?
Comment 5 David Cook 2016-05-31 02:48:49 UTC
(In reply to Srdjan Jankovic from comment #4)
> I think you should have included js removal with the patch, not used so
> should go.
> What's the story with the intranet basket?

Good points. It looks like the intranet has the same issue as the OPAC basket, so I should change that...
Comment 6 Jonathan Druart 2016-06-10 15:35:24 UTC
Same as Srdjan :)
So please:
1/ apply the same fix for intranet
2/ remove the JS function
3/ remove the code related to the print param

Additional comments:
4/ With your patch, the first column is now printed (checkbox)
5/ I only see 8 (out of 20) records printed
Comment 7 David Cook 2016-06-28 05:00:21 UTC
Thanks for looking at this Jonathan :).

I don't have time to work on this right now, but I'll keep it in mind for the future.
Comment 8 David Cook 2016-10-27 23:30:01 UTC
Can't believe it's almost been 6 months. This is still on my to do list although I don't think I'll be looking at it until 2017 at the earliest.
Comment 9 Owen Leonard 2018-05-30 14:20:37 UTC
Created attachment 75664 [details] [review]
Bug 16575: Irregular behaviour using window.print() followed by window.location.href

This patch updates the OPAC and staff client carts to use CSS to
control print output, removing a print parameter which was passed to the
script.

Currently, when you click "Print" on the OPAC basket, it navigates to
a new page and initiates window.print() followed by a
window.location.href change again. Unfortunately, due to differences in
IE, Chrome, and FF, it will either show the print options, navigate away
without showing them, or refuse to navigate away after printing. By
changing to using print CSS, we don't navigate away from the basket in
the first place, so we prevent this irregular behavior.

TEST PLAN

1) Apply the patch
2) Create an OPAC basket by clicking "Add to cart" on multiple items
3) Using Chrome, IE, and Firefox (of any version), click the "Print"
   button
4) You should see the relevant print menu without the OPAC basket
   re-loading in any way.
5) After printing is complete, you should still be on the OPAC basket
   pop-up
6) Perform the same tests in the staff client
Comment 10 Owen Leonard 2018-05-30 14:20:40 UTC
Created attachment 75665 [details] [review]
Bug 16575: (follow-up) Compiled CSS

This patch contains the compiled CSS for the OPAC print stylesheet.
Comment 11 Katrin Fischer 2018-07-08 16:11:08 UTC
Created attachment 76768 [details] [review]
Bug 16575: Irregular behaviour using window.print() followed by window.location.href

This patch updates the OPAC and staff client carts to use CSS to
control print output, removing a print parameter which was passed to the
script.

Currently, when you click "Print" on the OPAC basket, it navigates to
a new page and initiates window.print() followed by a
window.location.href change again. Unfortunately, due to differences in
IE, Chrome, and FF, it will either show the print options, navigate away
without showing them, or refuse to navigate away after printing. By
changing to using print CSS, we don't navigate away from the basket in
the first place, so we prevent this irregular behavior.

TEST PLAN

1) Apply the patch
2) Create an OPAC basket by clicking "Add to cart" on multiple items
3) Using Chrome, IE, and Firefox (of any version), click the "Print"
   button
4) You should see the relevant print menu without the OPAC basket
   re-loading in any way.
5) After printing is complete, you should still be on the OPAC basket
   pop-up
6) Perform the same tests in the staff client

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 12 Katrin Fischer 2018-07-08 16:11:13 UTC
Created attachment 76769 [details] [review]
Bug 16575: (follow-up) Compiled CSS

This patch contains the compiled CSS for the OPAC print stylesheet.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 13 Katrin Fischer 2018-07-08 16:11:35 UTC
I redid the second patch from scratch as it did no longer apply.
Comment 14 Marcel de Rooy 2018-07-13 06:48:32 UTC
> 5) After printing is complete, you should still be on the OPAC basket
>    pop-up

The popup is still open, but I did not return to it. (Chrome, FF)
Comment 15 Marcel de Rooy 2018-07-13 06:54:55 UTC
(In reply to Katrin Fischer from comment #13)
> I redid the second patch from scratch as it did no longer apply.

You made a mistake:
Patch 2 was:
From: Owen Leonard <oleonard@myacpl.org>
Date: Wed, 30 May 2018 14:18:43 +0000
Subject: [PATCH] Bug 16575: (follow-up) Compiled CSS

This patch contains the compiled CSS for the OPAC print stylesheet.
---
 koha-tmpl/opac-tmpl/bootstrap/css/print.css | 2 +-

But it became (you changed ANOTHER file):

From: Katrin Fischer <katrin.fischer.83@web.de>
Date: Sun, 8 Jul 2018 01:21:23 +0000
Subject: [PATCH 2/2] Bug 16575: (follow-up) Compiled CSS

This patch contains the compiled CSS for the OPAC print stylesheet.
---
 koha-tmpl/opac-tmpl/bootstrap/css/opac.css | 2 +-
Comment 16 Katrin Fischer 2018-07-13 07:38:27 UTC
Hm, what I did was skipping the second original patch as it didn't apply and running the opac-less command - strange that I came out with a changed file anyway. I would have expected an empty file. And also a bit strange that the print file was not regenerated, have to investigate.

Thx for noticing!
Comment 17 Katrin Fischer 2018-07-13 07:44:02 UTC
not an empty file, but no changed in the one I actually committed...
sorry for being unclear.
Comment 18 Katrin Fischer 2018-07-15 18:01:43 UTC
Created attachment 76967 [details] [review]
Bug 16575: (follow-up) Compiled CSS

This patch contains the compiled CSS for the OPAC print stylesheet.
Comment 19 Katrin Fischer 2018-07-15 18:02:07 UTC
I fixed the CSS.
Comment 20 Marcel de Rooy 2018-07-16 08:10:17 UTC
Created attachment 76971 [details] [review]
Bug 16575: Irregular behaviour using window.print() followed by window.location.href

This patch updates the OPAC and staff client carts to use CSS to
control print output, removing a print parameter which was passed to the
script.

Currently, when you click "Print" on the OPAC basket, it navigates to
a new page and initiates window.print() followed by a
window.location.href change again. Unfortunately, due to differences in
IE, Chrome, and FF, it will either show the print options, navigate away
without showing them, or refuse to navigate away after printing. By
changing to using print CSS, we don't navigate away from the basket in
the first place, so we prevent this irregular behavior.

TEST PLAN

1) Apply the patch
2) Create an OPAC basket by clicking "Add to cart" on multiple items
3) Using Chrome, IE, and Firefox (of any version), click the "Print"
   button
4) You should see the relevant print menu without the OPAC basket
   re-loading in any way.
5) After printing is complete, you should still be on the OPAC basket
   pop-up
6) Perform the same tests in the staff client

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 21 Marcel de Rooy 2018-07-16 08:10:22 UTC
Created attachment 76972 [details] [review]
Bug 16575: (follow-up) Compiled CSS

This patch contains the compiled CSS for the OPAC print stylesheet.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 22 Marcel de Rooy 2018-07-16 08:10:53 UTC
(In reply to Katrin Fischer from comment #19)
> I fixed the CSS.

Thanks.
Comment 23 Nick Clemens 2018-07-19 17:38:43 UTC
Awesome work all!

Pushed to master for 18.11
Comment 24 Martin Renvoize 2018-08-02 13:38:59 UTC
Pushed to 18.05.x for 18.05.03