Closed
Bug 1409971
Opened 7 years ago
Closed 7 years ago
Cancelling the print job doesn't work on GTK / Windows.
Categories
(Core :: Printing: Setup, defect, P3)
Tracking
()
RESOLVED
FIXED
mozilla59
People
(Reporter: mantaroh, Assigned: mantaroh)
References
Details
Attachments
(3 files)
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.
Comment 1•7 years ago
|
||
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.)
status-firefox57:
--- → wontfix
Flags: needinfo?(mantaroh)
Assignee | ||
Comment 2•7 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 | ||
Comment 4•7 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•7 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•7 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•7 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•7 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•7 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!
Assignee | ||
Comment 13•7 years ago
|
||
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d00b7e14a816201ed10c566b2affd12346d40938
Comment 14•7 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
Comment 15•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/983d715192fe https://hg.mozilla.org/mozilla-central/rev/154ec3c19c88 https://hg.mozilla.org/mozilla-central/rev/b78cea997533
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in
before you can comment on or make changes to this bug.
Description
•