Closed Bug 1654684 Opened 2 years ago Closed 2 years ago

Implement a paginator for the print preview

Categories

(Toolkit :: Printing, enhancement, P1)

Product:
Toolkit
Component:
Printing
Type:
enhancement

Tracking

()

Status:
VERIFIED FIXED
Milestone:
85 Branch
Tracking Flags:
Tracking Status
firefox85 --- verified
firefox86 --- verified

People

(Reporter: mstriemer, Assigned: sfoster)

References

(Regressed 2 open bugs)

Details

(Whiteboard: [print2020_v85])

Attachments

(2 files, 1 obsolete file)

Bug 1654684 - Add sheet-indicator and preview navigation to the print preview. r?mstriemer!
47 bytes, text/x-phabricator-request
Details | Review
Bug 1654684 - Move chevron.svg to toolkit. r?ntim
47 bytes, text/x-phabricator-request
Details | Review

There should be a paginator overlayed onto the print preview that shows the total number of pages and allows the user to skip to the next page, previous page, beginning, or end.

This could be done in the print.html document and positioned above the preview, or possibly in the preview document itself.

(Update from Sean: "Also, FWIW, Romain and Martin agreed that the paginator can be removed from scope for 81.")

Whiteboard: [print2020_v82]
Attachment #9168901 - Attachment is obsolete: true

FWIW nsIWebBrowserPrint's printPreviewNumPages, printPreviewCurrentPageNumber and printPreviewScrollToPage should all be working now.

[Moving front-end printing v82 bugs to v83.]

Whiteboard: [print2020_v82] → [print2020_v83]

It shouldn't be too much more work to get actual pagination here, but the first priority is to get a 1 of n sheets indicator on the preview.
I'm not sure how to handle duplex printing, I guess initially we'll just label those as new sheets and follow-up as necessary.

Assignee: nobody → sfoster
Status: NEW → ASSIGNED
Whiteboard: [print2020_v83] → [print2020_v84]
Priority: P2 → P1
Attachment #9187193 - Attachment description: Bug 1654684 - Add page-indicator control to the print preview → Bug 1654684 - Add sheet-indicator and preview navigation to the print preview. r?mstriemer!
Attachment #9188502 - Attachment description: Bug 1654684 - Move chevron.svg to toolkit. r?Gijs → Bug 1654684 - Move chevron.svg to toolkit. r?ntim
Whiteboard: [print2020_v84] → [print2020_v85]

I'm getting a test failure from the new browser_preview_navigation.js. It looks like there's a crash somewhere between testPreviewScroll and testPreviewNavigationCommands and the print tabmodal dialog goes away. testPreviewScroll does both focus and synthesizeKey stuff to page down the preview, as well as synthesized mouse events to scroll down. So I tried some voodoo in there to put it back into a good state but no change. Does anything suggest itself from log or the test? I'm drawing a blank...

https://treeherder.mozilla.org/jobs?repo=try&revision=6354c3f3d233b238d937f48b787ddb27f81ac669

Flags: needinfo?(mstriemer)
Blocks: 1681026
Pushed by sfoster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/87d06a4d6ab6
Move chevron.svg to toolkit. r=remote-protocol-reviewers,desktop-theme-reviewers,ntim
https://hg.mozilla.org/integration/autoland/rev/5a0333ca8a19
Add sheet-indicator and preview navigation to the print preview. r=mstriemer,fluent-reviewers,flod

Thanks for the backout. I can't reproduce locally yet, but am investigating.

Flags: needinfo?(sfoster)

(In reply to Sam Foster [:sfoster] (he/him) from comment #9)

I'm getting a test failure from the new browser_preview_navigation.js. It looks like there's a crash somewhere between testPreviewScroll and testPreviewNavigationCommands and the print tabmodal dialog goes away.

I was able to narrow this down to testPreviewNavigationCommands, and attempted to land this with that particular test skipped (tracked in Bug 1681026). testPreviewNavigationCommands exercises the home/back/next/end buttons and doesnt fundamentally do anything unique that isn't done in the other tests, so I'm not sure yet what the problem is there.

Pushed by sfoster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8ae1d037aa9c
Move chevron.svg to toolkit. r=remote-protocol-reviewers,desktop-theme-reviewers,ntim
https://hg.mozilla.org/integration/autoland/rev/2fdc1b5569e1
Add sheet-indicator and preview navigation to the print preview. r=mstriemer,fluent-reviewers,flod
Pushed by sfoster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3f892edf0139
Move chevron.svg to toolkit. r=remote-protocol-reviewers,desktop-theme-reviewers,ntim
https://hg.mozilla.org/integration/autoland/rev/519c764753f3
Add sheet-indicator and preview navigation to the print preview. r=mstriemer,fluent-reviewers,flod

The DeferredTask we use to debounce the scroll handling is now disarmed when the actor is destroyed (which happens when the print dialog closes) Furthermore, using the actor properties to register that scroll handler had the unintended consequence that any content scroll event would construct and enter the PrintingChild actor. We now add the scroll event listener to the contentWindow and remove it from within the actor's own lifecycle.

This was reproducing on mochitest-chrome tests on osx, so with this clean try run I'm re-landing the patch: https://treeherder.mozilla.org/jobs?repo=try&revision=2527ba2cf416bd000329340b818faa7548689d60

Flags: needinfo?(sfoster)

I think we've figured out the test failure and there's a patch on bug 1681026, so I'm clearing the ni on :mstriemer.

Flags: needinfo?(mstriemer)

https://hg.mozilla.org/mozilla-central/rev/3f892edf0139
https://hg.mozilla.org/mozilla-central/rev/519c764753f3

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
status-firefox85: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 85 Branch
Regressions: 1681908

Hello,

Confirming this a verified fixed on 86.0a1(20201218095607) and 85.0b3(20201217185930) with macOS 10.14.6, Win10x64 and Ubuntu 20.04

Status: RESOLVED → VERIFIED
status-firefox85: fixed → verified
status-firefox86: --- → verified
Flags: qe-verify+
Regressions: 1685328
You need to log in before you can comment on or make changes to this bug.