1697226 - Please add an option to print the current page only to the new printing UI

Status: RESOLVED FIXED

(Toolkit :: Printing, defect, P3)

Description

[Julien Wajsberg [:julienw]]

Attachment: Current options

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.

Comment 1

[Julien Wajsberg [:julienw]]

Attachment: Native print dialog (French Linux)

Comment 2

[Blake Winton (:bwinton) (:☕️)]

Seems like a good idea. I'll throw it in the backlog, and we'll see when we can get to it.

Comment 3

[Sam Foster [:sfoster] (he/him)]

I would be happy to mentor if someone wanted to put a patch together to add this.

Comment 4

[Julien Wajsberg [:julienw]]

(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

[Sam Foster [:sfoster] (he/him)]

(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).

Comment 6

[Tawah Peggy]

Attachment: Bug 1697226 - Add current option to the number of pages to be printed.r?Sam Foster.

please add an option to print the current page only to the new printing UI.

Comment 7

[Tawah Peggy]

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

[Tawah Peggy]

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

[Sam Foster [:sfoster] (he/him)]

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

[Tawah Peggy]

(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 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.
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

[Sam Foster [:sfoster] (he/him)]

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?

Comment 12

[Tawah Peggy]

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.

Comment 13

[Julien Wajsberg [:julienw]]

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

[Sam Foster [:sfoster] (he/him)]

(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

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 16

[Ramya Praneetha :ramya]

Attachment: Bug 1697226 - Added option to print current page. r?sfoster

Added the option to print the current page to print UI.
Added test to check if current page is selected correctly.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b9f534734c7b
Added option to print current page. r=sfoster,fluent-reviewers,flod,mstriemer

Comment 18

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/b9f534734c7b

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Is this something we want to call out in the relnotes for 105?

Comment 20

[Sylvestre Ledru [:Sylvestre]]

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)]:

Comment 21

[Ryan VanderMeulen [:RyanVM]]

Added to the Nightly 105 relnotes.