Add platform support to check if page range exceeds the current number of pages
Categories
(Core :: Print Preview, enhancement)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox85 | --- | fixed |
People
(Reporter: emmamalysz, Assigned: emilio)
Details
Attachments
(3 files)
We are trying to handle non-contiguous page ranges in Bug 499640. A problem is that we can have a custom range, update a setting that changes the total number of pages (such as scale or orientation), and our page range is now out of range.
If that is the case and we update the preview with the new settings change, we display a blank page in print preview. We should instead probably display all pages and the page range input will be invalidated from print.js.
If a user changes one of those settings and immediately prints (before the form had a chance to register the num-pages update and recognize it was invalid), we should probably print all of the pages.
|
Assignee | |
Comment 1•3 years ago
|
||
So, the behavior you want is something like "If the page ranges cause us to never print any page, then print all pages", or something of that sort, right?
Looking a bit into this, it's a bit unfortunate because by the time we do the skipping we don't know the final number of pages yet. We could potentially tag them later or something.
|
|
Assignee | |
Comment 2•3 years ago
|
||
So... I looked into it and it'd be somewhat ugly in the sense that we'd have to wait all the way for reflow to happen to decide how to skip the pages. And if we change that decision then we need to reflow again.
Would it be better to give the front-end the total number of displayed pages? Then it could check for it being zero, or something of that sort, and decide to not show the current preview, and update it as it sees fit.
|
|
Assignee | |
Comment 3•3 years ago
|
||
|
||
Updated•3 years ago
|
|
|
Assignee | |
Comment 4•3 years ago
|
||
This will allow them to react however they want to empty page ranges as
a result of another setting change.
Depends on D98182
|
|
Assignee | |
Comment 5•3 years ago
|
||
This returns an isEmpty boolean as a result of the printPreview() call. My understanding is that that would allow you to either not hide the rendering indicator and trigger another preview after updating the validity of the fields, or something else.
|
|
Assignee | |
Comment 6•3 years ago
|
||
(Let me know if that somehow wouldn't work for you, or ends up being too cumbersome and we can try to do something else).
|
||
Comment 7•3 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)
Created attachment 9190275 [details]
Bug 1679706 - Communicate to the front-end whether there are no visible pages at all. r=jfkthame,jwattThis will allow them to react however they want to empty page ranges as
a result of another setting change.
Wouldn't checking for sheetCount == 0 serve for this purpose?
|
|
Assignee | |
Comment 8•3 years ago
|
||
(In reply to Jonathan Kew (:jfkthame) from comment #7)
Wouldn't checking for
sheetCount == 0serve for this purpose?
No, because we create one (empty) sheet, just with no pages.
|
|
||
Comment 9•3 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #8)
(In reply to Jonathan Kew (:jfkthame) from comment #7)
Wouldn't checking for
sheetCount == 0serve for this purpose?No, because we create one (empty) sheet, just with no pages.
That seems wrong, no?
|
|
Assignee | |
Comment 10•3 years ago
|
||
(In reply to Jonathan Kew (:jfkthame) from comment #9)
That seems wrong, no?
Why? It seems reasonable enough to me but I might be missing something :)
|
|
||
Comment 11•3 years ago
|
||
If there are no pages to print, I'd have expected us to create zero sheets.
|
|
Assignee | |
Comment 12•3 years ago
|
||
Well... given our frame tree setup we need at least one sheet to deal with pages. We could somehow hide it either at paint time or at some other time after reflow if we end up in this situation, though it doesn't seem particularly trivial to do.
|
Reporter | |
Comment 13•3 years ago
|
||
Yeah, I think an isEmpty boolean would be helpful. Right now, I'm just comparing the totalPageCount to the stored endRange value of the custom range, but I didn't like how we were accessing a custom element's variable within our PrintEventhandler.
If isEmpty is true, I believe we could just dispatch a print settings event and set pageRanges to all like this:
document.dispatchEvent(
new CustomEvent("update-print-settings", {
bubbles: true,
detail: { pageRanges: [] },
})
);
|
||
Comment 14•3 years ago
|
||
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0bd6c96f1280 Cleanup a bit the page-sequence-using methods in nsPrintJob. r=jfkthame https://hg.mozilla.org/integration/autoland/rev/30bf6b9a836e Communicate to the front-end whether there are no visible pages at all. r=jfkthame
|
||
Comment 15•3 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/0bd6c96f1280
https://hg.mozilla.org/mozilla-central/rev/30bf6b9a836e
|
|
Assignee | |
Comment 16•3 years ago
|
||
This will get tested when the front-end uses the value.
|
|
||
Comment 17•3 years ago
|
||
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3f6e90beb51e Fix an obvious typo in nsPrintJob::GetIsEmpty().
|
||
Comment 18•3 years ago
|
||
| bugherder | ||
Description
•