Bug 38072 - Regression with modalPrint
Summary: Regression with modalPrint
Status: Passed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low critical
Assignee: Martin Renvoize (ashimema)
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-10-03 12:41 UTC by Martin Renvoize (ashimema)
Modified: 2025-11-28 07:54 UTC (History)
7 users (show)

See Also:
GIT URL:
Initiative type: ---
Sponsorship status: ---
Comma delimited list of Sponsors:
Crowdfunding goal: 0
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes a regression when printing dialogue boxes in certain Chromium-based browsers, for example, when printing the cashup summary for the point of sale system. Sometimes the print dialog failed to open, and instead you were faced with a flash of white before the new tab automatically closed and didn't print.
Version(s) released in:
Circulation function:


Attachments
Bug 38072: Fix regression in modal_printer (1.72 KB, patch)
2024-10-03 14:44 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38072: [alternate] Fix regression in modal print (1.74 KB, patch)
2024-10-07 15:51 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 38072: Fix regression in modal_printer (1.78 KB, patch)
2024-10-08 06:08 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 38072: Ensure the printing tab closes (1.31 KB, patch)
2024-10-08 06:08 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 38072: Fix regression in modal_printer (1.78 KB, patch)
2025-11-26 16:48 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 38072: Ensure the printing tab closes (1.21 KB, patch)
2025-11-26 16:48 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 38072: Fix regression in modal_printer (1.83 KB, patch)
2025-11-26 18:46 UTC, David Nind
Details | Diff | Splinter Review
Bug 38072: Ensure the printing tab closes (1.25 KB, patch)
2025-11-26 18:46 UTC, David Nind
Details | Diff | Splinter Review
Bug 38072: Fix regression in modal_printer (2.07 KB, patch)
2025-11-28 07:53 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 Martin Renvoize (ashimema) 2024-10-03 12:41:06 UTC
We have a few instanced of modalPrint in Koha and they appear to have all ceased to function.

returns (bundle modal) and cash registers (cashup summary) are both affected.
Comment 1 Martin Renvoize (ashimema) 2024-10-03 14:44:25 UTC
Created attachment 172360 [details] [review]
Bug 38072: Fix regression in modal_printer

This patch updates the JS for the modal printer to make it more
reliable.

I found that in recent versions of chrome the print dialog failed to
open and instead you were just faced with a flash of white before the
new tab autoclosed and no print takes place.

Test plan
1) Enable cash registers and point of sale preferences
2) Add a debit type and mark it as 'Can be sold'
3) Add a cash register
4) Use 'Point of sale' to sell one of the above item for cash against
   the register you just created.
5) Navigate to the register and 'cashup'
6) Click the 'summary' link to display a modal
7) User the 'Print' option in the modal and confirm you face a print
   dialog and can print
Comment 2 CJ Lynce 2024-10-04 00:22:12 UTC
Prior to applying the patch (on main), when clicking "Print" on the modal, a new tab is opened with the print data, along with the print dialog. Successfully printing or cancelling closes the print data tab, and returns you to the screen with modal. (Tested on Chrome, Edge, Firefox, and Brave)

After applying the patch, when click "Print" on the modal, a new tab is opened with the print data and print dialog (similar to before) but successfully printing, or cancelling, DOES NOT close the new tab in Chrome, Edge, or Brave (Chromium based.) It DOES close the new tab on Firefox.

Is this intended?

(Firefox v 131.0; Chrome 129.0.6668.90; Edge 129.0.2792.65; Brave 1.70.123 based on Chromium 129.0.6668.89)
Comment 3 Martin Renvoize (ashimema) 2024-10-07 14:35:08 UTC
That's interesting.. for me prior to the patch chrome would flash open the tab and immediately close it without triggering the print dialogue.

After the patch the correct behaviour I'm seeing is exactly what you're seeing prior to the patch.  i.e. opens tab, opens print dialog, on close or print closes tab.

I'm on Chrome 129.0.6668.89 (Official Build) (64-bit)
Comment 4 Lucas Gass (lukeg) 2024-10-07 15:13:25 UTC
(In reply to CJ Lynce from comment #2)
> Prior to applying the patch (on main), when clicking "Print" on the modal, a
> new tab is opened with the print data, along with the print dialog.
> Successfully printing or cancelling closes the print data tab, and returns
> you to the screen with modal. (Tested on Chrome, Edge, Firefox, and Brave)
> 
> After applying the patch, when click "Print" on the modal, a new tab is
> opened with the print data and print dialog (similar to before) but
> successfully printing, or cancelling, DOES NOT close the new tab in Chrome,
> Edge, or Brave (Chromium based.) It DOES close the new tab on Firefox.
> 
> Is this intended?
> 
> (Firefox v 131.0; Chrome 129.0.6668.90; Edge 129.0.2792.65; Brave 1.70.123
> based on Chromium 129.0.6668.89)

With the patch applied I experience the same thing as CJ when using Chrome ( Version 129.0.6668.58 (Official Build) (64-bit) )

In Firefox it is working as intended.
Comment 5 Lucas Gass (lukeg) 2024-10-07 15:18:04 UTC
> With the patch applied I experience the same thing as CJ when using Chrome (
> Version 129.0.6668.58 (Official Build) (64-bit) )
> 
> In Firefox it is working as intended.

Sorry, not enough morning coffee. I had this backwards:

-Works in Chrome ( Version 129.0.6668.58 (Official Build) (64-bit) )
-Does not close the tab in Firefox ( 131.0 (64-bit) )
Comment 6 Lucas Gass (lukeg) 2024-10-07 15:51:36 UTC
Created attachment 172483 [details] [review]
Bug 38072: [alternate] Fix regression in modal print

Test plan
1) Enable cash registers and point of sale preferences
2) Add a debit type and mark it as 'Can be sold'
3) Add a cash register
4) Use 'Point of sale' to sell one of the above item for cash against
   the register you just created.
5) Navigate to the register and 'cashup'
6) Click the 'summary' link to display a modal
7) User the 'Print' option in the modal and confirm you face a print
   dialog and can print
Comment 7 Lucas Gass (lukeg) 2024-10-07 15:52:22 UTC
CJ, can you test with my alternate patch instead?
Comment 8 CJ Lynce 2024-10-07 16:13:44 UTC
Lucas, everything works as expected with your alternate patch, in all browsers (print dialog appears, temp tab closes when finished).

FYI- I could only apply your alt patch on top of Martin's, not on stock main.
Comment 9 Victor Grousset/tuxayo 2024-10-07 20:23:57 UTC
I can't reproduce the issue on Chromium 129.0.6668.70 :(
Regardless of having my adblocker enabled or not.
The print tab as expected is closed when printing or canceling the browser print menu. (Same in Firefox)

Anyway, we could move on with CJ's testing.

But:

> I could only apply your alt patch on top of Martin's, not on stock main.

Indeed, the alternate patch diff shows that it's one top of the changes of the 1st patch.

Is the alternate patch a follow-up to complete the 1st one?
Comment 10 Lucas Gass (lukeg) 2024-10-07 20:37:35 UTC
(In reply to Victor Grousset/tuxayo from comment #9)
> I can't reproduce the issue on Chromium 129.0.6668.70 :(
> Regardless of having my adblocker enabled or not.
> The print tab as expected is closed when printing or canceling the browser
> print menu. (Same in Firefox)
> 
> Anyway, we could move on with CJ's testing.
> 
> But:
> 
> > I could only apply your alt patch on top of Martin's, not on stock main.
> 
> Indeed, the alternate patch diff shows that it's one top of the changes of
> the 1st patch.
> 
> Is the alternate patch a follow-up to complete the 1st one?

Yes, you'll need to apply both.
Comment 11 Victor Grousset/tuxayo 2024-10-08 00:02:08 UTC
> I can't reproduce the issue on Chromium 129.0.6668.70 :(
> [...]
> The print tab as expected is closed when printing
> or canceling the browser print menu. (Same in Firefox)

At least I see no change in behavior with the patches.
Comment 12 David Nind 2024-10-08 00:18:54 UTC
I can't replicate the issue - everything works the same for me both before and after the patches.

Tested on Ubuntu 22.04 - Firefox (Normal 131.0, Developer Edition 132.0b4), Chromium (129.0.6668.89), and Google Chrome (129.0.6668.89).

Testing notes:
1. Enable the cash registers and point of sale system preferences:
   - EnablePointOfSale: Enable
   - UseCashRegisters: Use
2. Add a debit type and mark it as 'Can be sold' (Administration > Accounting > Debit types> New debit type).
3. Add a cash register (Administration > Accounting > Cash registers > New cash register).
4. Use the point of sale module to sell one of the items created for cash against the register you just created (Point of sale > click 'Add' for the item you created > Collect payment > enter an amount for 'Amount tendered').
5. Navigate to the register and 'cashup' (Point of sale > cash management > Cash summary for Centerville > Record cashup action, then 'Confirm').
6. Click the 'summary' link to display a modal (in the 'Last cashup' column for the cash register).
7. For the Koha cashup summary dialog box, click 'Print'. A standard device/system print dialog box should appear.
Comment 13 Victor Grousset/tuxayo 2024-10-08 06:08:37 UTC
Created attachment 172508 [details] [review]
Bug 38072: Fix regression in modal_printer

This patch updates the JS for the modal printer to make it more
reliable.

I found that in recent versions of chrome the print dialog failed to
open and instead you were just faced with a flash of white before the
new tab autoclosed and no print takes place.

Test plan
1) Enable cash registers and point of sale preferences
2) Add a debit type and mark it as 'Can be sold'
3) Add a cash register
4) Use 'Point of sale' to sell one of the above item for cash against
   the register you just created.
5) Navigate to the register and 'cashup'
6) Click the 'summary' link to display a modal
7) Use the 'Print' option in the modal and confirm you face a print
   dialog and can print
8) The print tab should be closed and you should be back to Koha
Comment 14 Victor Grousset/tuxayo 2024-10-08 06:08:40 UTC
Created attachment 172509 [details] [review]
Bug 38072: Ensure the printing tab closes
Comment 15 Victor Grousset/tuxayo 2024-10-08 06:12:36 UTC
Comment on attachment 172483 [details] [review]
Bug 38072: [alternate] Fix regression in modal print

I tried to clarify the role of the 2nd patch.
Comment 16 Victor Grousset/tuxayo 2024-10-08 06:19:06 UTC
And I realized even CJ their testing didn't got the main issue of this ticket.

Any more details Martin of how you get the print dialog to fail?
That's now 4 people who didn't got the completely broken print dialog with the current test plan. There must be something else needed to throw things off ^^"
Comment 17 Martin Renvoize (ashimema) 2024-10-08 14:08:57 UTC
I have no idea how to replicate it.. for me it happens consistently with and without any extensions installed.

It's odd.. browser hell.. I just assumed it was happening for anyone else using the same Chrome version, but that doesn't even appear to be the case.
Comment 18 Victor Grousset/tuxayo 2024-10-09 21:26:22 UTC
If you know about someone else having the issue and not having it anymore when the patch is there, then we can go with that. (testing on a sandbox with and without the patch)

Since it seems safe to say the patch doesn't introduce a regression, we could do that for these hard to reproduce issues.
Comment 19 Victor Grousset/tuxayo 2024-10-24 04:00:20 UTC
> If you know about someone else having the issue and not having it anymore when the patch is there, then we can go with that.

So needing one report of having seen the fix work. Whether locally or in production without knowing what are the exact conditions there that we are missing here to reproduce. At least being certain that the fix can fix :)

An alternative is to treat this as a refactor and go with it now. And cross fingers that it will fix it for people having the bug.
Comment 20 Martin Renvoize (ashimema) 2025-11-10 08:08:31 UTC
This is pretty annoying.. it still happens for me consistently in Chrome (Now 142.0.7444.134)
Comment 21 Martin Renvoize (ashimema) 2025-11-10 08:18:48 UTC
This is a timing-dependent race condition that manifests based on:
  - Chrome version (I'm on 142 now)
  - Print dialog type (Chrome preview vs system dialog)
  - OS print subsystem timing
  - Machine speed

The original code is objectively flawed - it calls win.close() immediately after win.print() without waiting. Even if it works for some, it's racing with the browser's async print dialog and will fail under certain conditions.

Request for testers: Please try on Chrome 140+ with different print settings, or deliberately slow down your system to expose the race."
Comment 22 David Nind 2025-11-12 17:46:16 UTC
I've still had difficulty replicating the issue, but was going to sign off anyway if things still continued to work as expected.

However, the patch no longer applies 8-(...

git bz apply 38072

Bug 38072 - Regression with modalPrint

172508 - Bug 38072: Fix regression in modal_printer
172509 - Bug 38072: Ensure the printing tab closes

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 38072: Fix regression in modal_printer
Using index info to reconstruct a base tree...
M	koha-tmpl/intranet-tmpl/prog/js/modal_printer.js
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/js/modal_printer.js
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/js/modal_printer.js
error: Failed to merge in the changes.
Patch failed at 0001 Bug 38072: Fix regression in modal_printer
Comment 23 Nick Clemens (kidclamp) 2025-11-26 16:48:04 UTC
Created attachment 189970 [details] [review]
Bug 38072: Fix regression in modal_printer

This patch updates the JS for the modal printer to make it more
reliable.

I found that in recent versions of chrome the print dialog failed to
open and instead you were just faced with a flash of white before the
new tab autoclosed and no print takes place.

Test plan
1) Enable cash registers and point of sale preferences
2) Add a debit type and mark it as 'Can be sold'
3) Add a cash register
4) Use 'Point of sale' to sell one of the above item for cash against
   the register you just created.
5) Navigate to the register and 'cashup'
6) Click the 'summary' link to display a modal
7) Use the 'Print' option in the modal and confirm you face a print
   dialog and can print
8) The print tab should be closed and you should be back to Koha
Comment 24 Nick Clemens (kidclamp) 2025-11-26 16:48:07 UTC
Created attachment 189971 [details] [review]
Bug 38072: Ensure the printing tab closes
Comment 25 Nick Clemens (kidclamp) 2025-11-26 16:49:00 UTC
easy whitespace conflicts - rebased so it can be tested
Comment 26 David Nind 2025-11-26 18:46:51 UTC
Created attachment 189979 [details] [review]
Bug 38072: Fix regression in modal_printer

This patch updates the JS for the modal printer to make it more
reliable.

I found that in recent versions of chrome the print dialog failed to
open and instead you were just faced with a flash of white before the
new tab autoclosed and no print takes place.

Test plan
1) Enable cash registers and point of sale preferences
2) Add a debit type and mark it as 'Can be sold'
3) Add a cash register
4) Use 'Point of sale' to sell one of the above item for cash against
   the register you just created.
5) Navigate to the register and 'cashup'
6) Click the 'summary' link to display a modal
7) Use the 'Print' option in the modal and confirm you face a print
   dialog and can print
8) The print tab should be closed and you should be back to Koha

Signed-off-by: David Nind <david@davidnind.com>
Comment 27 David Nind 2025-11-26 18:46:54 UTC
Created attachment 189980 [details] [review]
Bug 38072: Ensure the printing tab closes

Signed-off-by: David Nind <david@davidnind.com>
Comment 28 David Nind 2025-11-26 18:54:06 UTC
Testing notes (using KTD):

1. As noted in comment #22, I couldn't replicate the issue (using Chromium 142.0.7444.59 or Google Chrome 142.0.7444.175 on Ubuntu 22.04).

2. Things work as expected after the patches.
Comment 29 Marcel de Rooy 2025-11-28 07:53:54 UTC
Created attachment 190008 [details] [review]
Bug 38072: Fix regression in modal_printer

This patch updates the JS for the modal printer to make it more
reliable.

I found that in recent versions of chrome the print dialog failed to
open and instead you were just faced with a flash of white before the
new tab autoclosed and no print takes place.

Test plan
1) Enable cash registers and point of sale preferences
2) Add a debit type and mark it as 'Can be sold'
3) Add a cash register
4) Use 'Point of sale' to sell one of the above item for cash against
   the register you just created.
5) Navigate to the register and 'cashup'
6) Click the 'summary' link to display a modal
7) Use the 'Print' option in the modal and confirm you face a print
   dialog and can print
8) The print tab should be closed and you should be back to Koha

Signed-off-by: David Nind <david@davidnind.com>

Bug 38072: Ensure the printing tab closes

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
[EDIT] Squashed and tidied.