Cancelling the print job doesn't work on GTK / Windows.

RESOLVED FIXED in Firefox 59

Status

()

defect
P3
normal
RESOLVED FIXED
2 years ago
Last year

People

(Reporter: mantaroh, Assigned: mantaroh)

Tracking

unspecified
mozilla59
x86
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 wontfix, firefox59 fixed)

Details

Attachments

(3 attachments)

Current print progress dialog of GTK is XUL dialog. When user push the cancel button of this dialog, gecko will mark print job settings to cancel.[1]
However this mark doesn't pass to the parent process on e10s environment, as result, we can't cancel the print job.

This phenomenon will occur on e10s only.

Note:
 * macOS doesn't have this problem due to doesn't have this progress dialog.
 * windows doesn't set this parameter, so I think windows has this problem before e10s. (Maybe bug 424965)

[1] http://searchfox.org/mozilla-central/rev/d0448c970093f94bd986a21d3a9e8c366b316eb4/toolkit/components/printingui/unixshared/nsPrintProgress.cpp#136

STR:
 1) Open long page(e.g. DisplayList.cpp of dxr).
 2) Click the Print or Print Preview
 3) Click cancel button of progress dialog

Actually Result:
 We can't cancel print job.

Expected Result:
 Pushing the cancel button affect canceling the print job.
Do you know if this has always been broken with e10s, vs. if this is a more recent regression?

(Regardless, calling this wontfix for 57 since the risk/reward tolerance for changes there is pretty strict at this point. Though if this happens to be a regression *in 57*, that might merit reconsideration.)
Flags: needinfo?(mantaroh)
Assignee

Comment 2

2 years ago
(In reply to Daniel Holbert [:dholbert] from comment #1)
> Do you know if this has always been broken with e10s, vs. if this is a more
> recent regression?
> 
> (Regardless, calling this wontfix for 57 since the risk/reward tolerance for
> changes there is pretty strict at this point. Though if this happens to be a
> regression *in 57*, that might merit reconsideration.)

As far as I can see, this phenomenon happened after bug 1090448 in my linux environment.
So I think this is not recent regression but bug of enabling e10s printing.
Flags: needinfo?(mantaroh)
Assignee

Updated

2 years ago
Duplicate of this bug: 1415347
Assignee

Comment 4

2 years ago
(In reply to Mantaroh Yoshinaga[:mantaroh] from comment #2)
> (In reply to Daniel Holbert [:dholbert] from comment #1)
> > Do you know if this has always been broken with e10s, vs. if this is a more
> > recent regression?
> > 
> > (Regardless, calling this wontfix for 57 since the risk/reward tolerance for
> > changes there is pretty strict at this point. Though if this happens to be a
> > regression *in 57*, that might merit reconsideration.)
> 
> As far as I can see, this phenomenon happened after bug 1090448 in my linux
> environment.
> So I think this is not recent regression but bug of enabling e10s printing.

I guess we need to send notification of cancelling the print job via IPC.

This is WIP patch:
https://hg.mozilla.org/try/rev/bb708e83e902f3fe7008f9c4338a631ac94ab634
Assignee: nobody → mantaroh
Assignee

Comment 5

2 years ago
(In reply to Mantaroh Yoshinaga[:mantaroh] from comment #4)
> (In reply to Mantaroh Yoshinaga[:mantaroh] from comment #2)
> > (In reply to Daniel Holbert [:dholbert] from comment #1)
> > > Do you know if this has always been broken with e10s, vs. if this is a more
> > > recent regression?
> > > 
> > > (Regardless, calling this wontfix for 57 since the risk/reward tolerance for
> > > changes there is pretty strict at this point. Though if this happens to be a
> > > regression *in 57*, that might merit reconsideration.)
> > 
> > As far as I can see, this phenomenon happened after bug 1090448 in my linux
> > environment.
> > So I think this is not recent regression but bug of enabling e10s printing.
> 
> I guess we need to send notification of cancelling the print job via IPC.
> 
> This is WIP patch:
> https://hg.mozilla.org/try/rev/bb708e83e902f3fe7008f9c4338a631ac94ab634

We need to following changes :
 * Open the traffic of cancelling job via ipc.
 * Have a member which reference to ipc parent.
 * Call this new ipc interface from nsPrintProgress::SetProcessCanceledByUser().
 * Receive notifying cancel job on child process, and should nsPrintSettings::isCancelled to true.
 * These change work only e10s environment.

Note:
 * nsPrintProgress() created by nsPrintingPromptService, so we can't pass ipc parent(like PrintingParent) pointer directly. So we need to pass XPCOM interface like nsIObserver. Actually PrintProgressDialog uses nsIObserver.[1]
 * We can use this nsIObserver(i.e. PrintProgressDialogParent).

[1] https://searchfox.org/mozilla-central/rev/a662f122c37704456457a526af90db4e3c0fd10e/toolkit/components/printingui/ipc/PrintProgressDialogParent.cpp#96
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 9

2 years ago
mozreview-review
Comment on attachment 8928424 [details]
Bug 1409971 - Part 1. Add cancelling print job interface to PPrintProgressDialog.

https://reviewboard.mozilla.org/r/199654/#review205628

Thanks!
Attachment #8928424 - Flags: review?(mconley) → review+

Comment 10

2 years ago
mozreview-review
Comment on attachment 8928425 [details]
Bug 1409971 - Part 2. Notify cancelling print job when cancel button pushed.

https://reviewboard.mozilla.org/r/199656/#review205630
Attachment #8928425 - Flags: review?(mconley) → review+

Comment 11

2 years ago
mozreview-review
Comment on attachment 8928426 [details]
Bug 1409971 - Part 3. Set nsIPrintSettings::IsCancelled to true in order to cancel print job.

https://reviewboard.mozilla.org/r/199658/#review205632

A very straight-forward set of patches. Thank you!
Attachment #8928426 - Flags: review?(mconley) → review+
Assignee

Comment 12

2 years ago
(In reply to Mike Conley (:mconley) (:⚙️) - Backlogged on reviews and needinfos from comment #11)
> Comment on attachment 8928426 [details]
> Bug 1409971 - Part 3. Set nsIPrintSettings::IsCancelled to true in order to
> cancel print job.
> 
> https://reviewboard.mozilla.org/r/199658/#review205632
> 
> A very straight-forward set of patches. Thank you!

Thanks!

Comment 14

2 years ago
Pushed by mantaroh@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/983d715192fe
Part 1. Add cancelling print job interface to PPrintProgressDialog. r=mconley
https://hg.mozilla.org/integration/autoland/rev/154ec3c19c88
Part 2. Notify cancelling print job when cancel button pushed. r=mconley
https://hg.mozilla.org/integration/autoland/rev/b78cea997533
Part 3. Set nsIPrintSettings::IsCancelled to true in order to cancel print job. r=mconley
Depends on: 1419775

Updated

Last year
Depends on: 1441598
You need to log in before you can comment on or make changes to this bug.