Closed Bug 1657763 Opened 6 months ago Closed 5 months ago

Refine the way to choose the current page

Categories

(Core :: Print Preview, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
81 Branch
Tracking Status
firefox81 --- fixed

People

(Reporter: hiro, Assigned: hiro)

References

(Blocks 1 open bug)

Details

(Whiteboard: [print2020_v81])

Attachments

(2 files)

In bug 1657515 we are going to add nsIWebBrowserPrint.printPreviewCurrentPageNumber, but the initial implementation to choose the current page number is primitive. The way is;

  1. Enumerate all pages from the first one to the last one
  2. If the page intersects with the print preview scroll port, it's chosen as the current one, even if the intersection area is the bottom 1px height of the page

Emilio suggested to choose the page where a half of the page is visible (that's said, I am suspecting this way doesn't work for heterogeneous page size case, it's possible there is no page where half of it is visible)

An alternative way what I have in my mind is to choose the closest page from the center of the scroll port.

Also, TYLin noticed me that once after we supported pages per a sheet option (bug 1631452), what should the current page number be?

mstriemer, could you please give us your insights on this issue from the frontend perspective?

Flags: needinfo?(mstriemer)
Summary: Refine the way to choose the current current page → Refine the way to choose the current page
Severity: -- → S3

(In reply to Hiroyuki Ikezoe (:hiro) from comment #0)

Also, TYLin noticed me that once after we supported pages per a sheet option (bug 1631452), what should the current page number be?

I think for scrolling to a page, the general expectation would be that the "page" number would be physical page (sheet) number. Actually, I've just checked what Chrome does and that is the case there.

Also I think it's shorlander that would be the person to ask about the behaviour of what page (i.e. sheet) number to choose. I also just checked what Chrome does there, and actually they seem to do what you suggest (the current page number displayed is the page that is interceted by or closest to the center of the print preview viewport). I guess that's contrary to my suggestion in bug 1657515 comment 2 (choosing the page intersected by/closest to the top edge of the viewport). But shorlander should decide this.

Flags: needinfo?(shorlander)

The middle of the viewport makes sense to me, but deferring to Stephen sounds good to me.

I put together a WIP patch in bug 1654684 to pull this value and when it's in landscape currently it won't update to the last page being current even at the bottom. I'd imagine that could happen with either of these cases though.

That patch also has a FIXME (with a stack trace) where this seems to throw an error when called immediately after the preview is updated.

Flags: needinfo?(mstriemer)
See Also: → 1657550

Stephen seems super busy, but when I pinged him he said:

Yeah, the bottom page that crosses the 50% mark would be the most visible page “current” page.

Hopefully that allows you to proceed, hiro?

Flags: needinfo?(hikezoe.birchill)
Attached image Current Page

(In reply to Jonathan Watt [:jwatt] from comment #2)

Also I think it's shorlander that would be the person to ask about the behaviour of what page (i.e. sheet) number to choose. I also just checked what Chrome does there, and actually they seem to do what you suggest (the current page number displayed is the page that is interceted by or closest to the center of the print preview viewport). I guess that's contrary to my suggestion in bug 1657515 comment 2 (choosing the page intersected by/closest to the top edge of the viewport). But shorlander should decide this.

Yeah that sounds right to me, whichever page is the most visible page should be considered the current page.

Flags: needinfo?(shorlander)

Thank you, both, I am going to take this.

Assignee: nobody → hikezoe.birchill
Status: NEW → ASSIGNED
Flags: needinfo?(hikezoe.birchill)
Whiteboard: [print2020_v81]

I've uploaded the fix for this issue, but I haven't had any great idea to write automated tests for this..

Attachment #9170263 - Attachment description: Bug 1657763 - Choose the closest page to the center of the scroll port as the "current" page. → Bug 1657763 - Choose the closest page to the center of the scroll port as the "current" page. r?emilio
Pushed by hikezoe.birchill@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8091fc7428c4
Choose the closest page to the center of the scroll port as the "current" page. r=emilio
Priority: -- → P2
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 81 Branch
You need to log in before you can comment on or make changes to this bug.