Refine the way to choose the current page
Categories
(Core :: Print Preview, enhancement, P2)
Tracking
()
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;
- Enumerate all pages from the first one to the last one
- 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?
Assignee | ||
Comment 1•6 months ago
|
||
mstriemer, could you please give us your insights on this issue from the frontend perspective?
Assignee | ||
Updated•6 months ago
|
Assignee | ||
Updated•6 months ago
|
Comment 2•6 months ago
|
||
(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.
Comment 3•6 months ago
|
||
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.
Comment 4•5 months ago
|
||
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?
Comment 5•5 months ago
|
||
(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.
Assignee | ||
Comment 6•5 months ago
|
||
Thank you, both, I am going to take this.
Updated•5 months ago
|
Assignee | ||
Comment 7•5 months ago
|
||
Assignee | ||
Comment 8•5 months ago
|
||
I've uploaded the fix for this issue, but I haven't had any great idea to write automated tests for this..
Updated•5 months ago
|
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
Updated•5 months ago
|
Comment 10•5 months ago
|
||
bugherder |
Description
•