Please add an option to print the current page only to the new printing UI
Categories
(Toolkit :: Printing, defect, P3)
Tracking
()
People
(Reporter: julienw, Assigned: ramya, Mentored)
Details
(Keywords: regression, Whiteboard: [print2020_later] [lang=js])
Attachments
(4 files)
The new printing UI has 2 options for the "Pages" option currently: "All" and "Custom". It would benefit greatly from adding a new option "Current page", like we have on the native print dialog (at least on Linux).
Also please note that it doesn't seem to work in the native print dialog anymore (the option is disabled), so I don't see any way to do it currently. This seems like a regression to me.
Reporter | ||
Updated•4 years ago
|
Reporter | ||
Comment 1•4 years ago
|
||
Reporter | ||
Updated•4 years ago
|
Comment 2•4 years ago
|
||
Seems like a good idea. I'll throw it in the backlog, and we'll see when we can get to it.
Updated•4 years ago
|
Comment 3•4 years ago
|
||
I would be happy to mentor if someone wanted to put a patch together to add this.
Reporter | ||
Comment 4•4 years ago
|
||
(In reply to Sam Foster [:sfoster] (he/him) from comment #3)
I would be happy to mentor if someone wanted to put a patch together to add this.
Cool :-) Do you know where the change should be done? Is that difficult to do cross-platform?
Comment 5•3 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #4)
Cool :-) Do you know where the change should be done? Is that difficult to do cross-platform?
We'd need to add the option in print.html and then something to handle it in print.js.
I don't think we currently expose the current page number, so we would need a way to get that (its in a current-page attribute on the preview <browser> element).
Updated•3 years ago
|
Comment 6•3 years ago
|
||
please add an option to print the current page only to the new printing UI.
Updated•3 years ago
|
Comment 7•3 years ago
|
||
Hello [:sfoster] I have submitted a work in progress patch for this.
please I searched for the FTL file to add the translation but I didn't see, I will appreciate if you can lead me to it.
also I don't quite know how to go about adding the JavaScript logic of the said functionality, please expantiate.
PS: I am new to contributing to this organization.
Comment 8•3 years ago
|
||
Hello Sam, I have made updates on the patch by adding a key to reference (Current) in printui.ftl . please check it out.
I am still trying to figure out the logic for printing the current page with respect to the way (all) and (custom) has been done, please help me with some insights.
Comment 9•3 years ago
|
||
Hi there,
Thanks for working on this. Currently the page-range control in the print UI has 2 possible values: "All" and "custom". We'd like to add a 3rd option: "Current". So, the first part is adding the new <option>. which you'll already got in your patch.
We currently represent "all pages" as an empty array on the front end, and a custom page range as an array of page numbers. I think probably the simplest solution for "current page" is to simply populate this array with the current page number. Its not very obvious right now how to get the current page number, or how that should work so let me make some suggestions:
First, poking around at the js state in the print dialog window is a bit awkward, so this is a helpful snippet to run in the browser console:
let win = PrintUtils.getPreviewBrowser(gBrowser.selectedBrowser).closest(".dialogBox").querySelector(".dialogFrame").contentWindow
that lets you more easily examine e.g. win.PrintEventHandler
, win.document.querySelector("#page-range-input")
etc.
The <browser> we use to display the print preview knows what the current page number is - we use that to update the preview pagination values. You can reach that via PrintEventHandler.printPreviewEl.querySelector(".printPreviewBrowser")
(PrintEventHandler is global in the print.html context). To make this value available to the PageRangeInput
instance, I would suggest adding it as a "virtual" property on the PrintSettingsViewProxy
object. This is the settings
object passed to each custom element's update()
method. I.e.
diff --git a/toolkit/components/printing/content/print.js b/toolkit/components/printing/content/print.js
--- a/toolkit/components/printing/content/print.js
+++ b/toolkit/components/printing/content/print.js
@@ -1252,6 +1252,12 @@ var PrintSettingsViewProxy = {
get(target, name) {
switch (name) {
+ case "currentPageNumber": {
+ let previewBrowser = PrintEventHandler.printPreviewEl.querySelector(".printPreviewBrowser");
+ let pageNum = parseInt(previewBrowser.getAttribute("current-page"));
+ return isNaN(pageNum) ? 1 : pageNum;
+ }
+
Then, in PageRangeInput's update
method:
let { pageRanges, printerName, currentPageNumber } = settings;
this._currentPage = currentPageNumber;
Now, when ._rangePicker
's value changes to "current" you can simply set this._rangeInput.value = this._currentPage
. It might look like a change to updatePageRange
like this:
- } else {
+ } else if (this._rangePicker.value == "current") {
+ this._rangeInput.value = this._currentPage;
+ this.validateRangeInput();
+ }
+ else {
I'd suggest we not show the range input when the range picker is "current". But, if the user does then select "custom", the value will be there to edit. This way we don't need to change any of the validation or any of the rest of the logic for how these range values are handled. And as the range is a single page, the user will not be able to scroll or paginate - which would change the current page number and confuse everything.
Hopefully that gets you unstuck. Eventually we'll want a new test to cover this functionality itself, but I would suggest working through getting it working first, in case we've missed something.
Comment 10•3 years ago
|
||
(In reply to Sam Foster [:sfoster] (he/him) from comment #9)
Hi there,
Thanks for working on this. Currently the page-range control in the print UI has 2 possible values: "All" and "custom". We'd like to add a 3rd option: "Current". So, the first part is adding the new <option>. which you'll already got in your patch.We currently represent "all pages" as an empty array on the front end, and a custom page range as an array of page numbers. I think probably the simplest solution for "current page" is to simply populate this array with the current page number. Its not very obvious right now how to get the current page number, or how that should work so let me make some suggestions:
First, poking around at the js state in the print dialog window is a bit awkward, so this is a helpful snippet to run in the browser console:
let win = PrintUtils.getPreviewBrowser(gBrowser.selectedBrowser).closest(".dialogBox").querySelector(".dialogFrame").contentWindow
that lets you more easily examine e.g.
win.PrintEventHandler
,win.document.querySelector("#page-range-input")
etc.The <browser> we use to display the print preview knows what the current page number is - we use that to update the preview pagination values. You can reach that via
PrintEventHandler.printPreviewEl.querySelector(".printPreviewBrowser")
(PrintEventHandler is global in the print.html context). To make this value available to thePageRangeInput
instance, I would suggest adding it as a "virtual" property on thePrintSettingsViewProxy
object. This is thesettings
object passed to each custom element'supdate()
method. I.e.diff --git a/toolkit/components/printing/content/print.js b/toolkit/components/printing/content/print.js --- a/toolkit/components/printing/content/print.js +++ b/toolkit/components/printing/content/print.js @@ -1252,6 +1252,12 @@ var PrintSettingsViewProxy = { get(target, name) { switch (name) { + case "currentPageNumber": { + let previewBrowser = PrintEventHandler.printPreviewEl.querySelector(".printPreviewBrowser"); + let pageNum = parseInt(previewBrowser.getAttribute("current-page")); + return isNaN(pageNum) ? 1 : pageNum; + } +
Then, in PageRangeInput's
update
method:let { pageRanges, printerName, currentPageNumber } = settings; this._currentPage = currentPageNumber;
Now, when
._rangePicker
's value changes to "current" you can simply setthis._rangeInput.value = this._currentPage
. It might look like a change toupdatePageRange
like this:- } else { + } else if (this._rangePicker.value == "current") { + this._rangeInput.value = this._currentPage; + this.validateRangeInput(); + } + else {
I'd suggest we not show the range input when the range picker is "current". But, if the user does then select "custom", the value will be there to edit. This way we don't need to change any of the validation or any of the rest of the logic for how these range values are handled. And as the range is a single page, the user will not be able to scroll or paginate - which would change the current page number and confuse everything.
Hopefully that gets you unstuck. Eventually we'll want a new test to cover this functionality itself, but I would suggest working through getting it working first, in case we've missed something.
Wow!! Thank you very much Sam.
This makes a lot of sense, let me get to it straight away and if I fall into any issues I will let you know.
Comment 11•3 years ago
|
||
Hey Julien, I also noticed the current page option is now disabled in the native dialog and looking around I'm not sure what exactly the behavior should be. I prototyped something to be able to write comment 9, where we use the current page from the print preview. As the tab modal sits on top of the content document the preview is both more visible and a more accurate representation of what the current page might look like once printed. But, setting the current page as the page range has the effect of recalculating the preview to only show that page. If you want to navigate the whole document again, you need to go back to all pages or custom range, scroll or hit next/back in the paginator and then select custom page again.
How would you expect this to work? We don't really have a way of previewing the whole document but only printing a single page at the moment, but if that's really the best answer we can file some more bugs and see if its doable. Or we can pursue the approach from comment #9 as an incremental improvement and necessary step to find the optimal behaviour. Or maybe there is a better well-defined answer already out there?
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Comment 12•3 years ago
|
||
hello Sam i have made updates to our patch, its almost there.
i have a couple of debugging issues though, one of which is logging things to the console.
let win = >PrintUtils.getPreviewBrowser(gBrowser.selectedBrowser).closest(".dialogBox").querySelector(".dialogFra>>me").contentWindow
this line returns printUtils is not known.
Also when I try to add some code in print.js so it gets printed to the console i don't see it printed there, this has made me quite unable to debug properly. please give me heads up.
Reporter | ||
Comment 13•3 years ago
|
||
hey Sam,
sorry for the delay as I was away for a few days :-)
(In reply to Sam Foster [:sfoster] (he/him) from comment #11)
Hey Julien, I also noticed the current page option is now disabled in the native dialog and looking around I'm not sure what exactly the behavior should be. I prototyped something to be able to write comment 9, where we use the current page from the print preview. As the tab modal sits on top of the content document the preview is both more visible and a more accurate representation of what the current page might look like once printed. But, setting the current page as the page range has the effect of recalculating the preview to only show that page. If you want to navigate the whole document again, you need to go back to all pages or custom range, scroll or hit next/back in the paginator and then select custom page again.
How would you expect this to work? We don't really have a way of previewing the whole document but only printing a single page at the moment, but if that's really the best answer we can file some more bugs and see if its doable. Or we can pursue the approach from comment #9 as an incremental improvement and necessary step to find the optimal behaviour. Or maybe there is a better well-defined answer already out there?
I think that what you propose in comment 9 makes sense. I believe that if in that case only the specified page is displayed, it also makes sense. I'm looking forward to it :-)
Thanks!
Comment 14•3 years ago
|
||
(In reply to Tawah Peggy from comment #12)
hello Sam i have made updates to our patch, its almost there.
i have a couple of debugging issues though, one of which is logging things to the console.let win = >PrintUtils.getPreviewBrowser(gBrowser.selectedBrowser).closest(".dialogBox").querySelector(".dialogFra>>me").contentWindow
this line returns printUtils is not known.
I suspect you are not in the right console. Make sure you are using the Browser Console not the regular devtools console. PrintUtils
may need to be lazy-loaded, and certainly the preview browser will not exist until the print UI is open.
Also when I try to add some code in print.js so it gets printed to the console i don't see it printed there, this has made me quite unable to debug properly. please give me heads up.
Yeah, definitely sounds like you are looking at the wrong console.
Comment 15•2 years ago
|
||
The bug assignee is inactive on Bugzilla, so the assignee is being reset.
Assignee | ||
Comment 16•2 years ago
|
||
Added the option to print the current page to print UI.
Added test to check if current page is selected correctly.
Updated•2 years ago
|
Comment 17•2 years ago
|
||
Comment 18•2 years ago
|
||
bugherder |
Updated•2 years ago
|
Comment 19•2 years ago
|
||
Is this something we want to call out in the relnotes for 105?
Comment 20•2 years ago
|
||
yeah, we should
Release Note Request (optional, but appreciated)
[Why is this notable]: New cool feature (everybody loves dealing with printers, right?)
[Affects Firefox for Android]: no
[Suggested wording]: Added an option to print only current page
[Links (documentation, blog post, etc)]:
Description
•