1661852 - Print range isn't honored after you change printers in the UI

Status: RESOLVED FIXED

(Toolkit :: Printing, defect, P2)

Description

[Daniel Holbert [:dholbert]]

STR:

1. Ensure you have print.tab_modal.enabled = true in about:config
2. View some multi-page document like https://www.mozilla.org/en-US/MPL/2.0/
3. Ctrl+P to open the tab-modal print dialog.
4. Choose some real printer from the dropdown at the top.
5. Choose print range from:2 to:3 (for example)
6. Change the printer at the top to be "Save to PDF".
   (Notice that the print UI still shows a custom page range, from:2 to:3)
7. Proceed with the save-to-PDF operation.

EXPECTED RESULTS:
I should end up with a PDF that only contains pages 2-3.

ACTUAL RESULTS:
I end up with a full-length PDF (all the pages)

I'm guessing we fail to initialize the print settings with the custom page range when we switch printers in the UI, or something (and we only do so lazily when the page range is updated).

Comment 1

[Daniel Holbert [:dholbert]]

Note: if you build with bug 1659005's patches, you'll encounter this bug in the preview UI as well -- that's where I noticed it first. But it's a version of the same bug, and it affects preview as well as actual printing.

(A local workaround is to just adjust the page range after you've changed the printer in step 6 - that gets the page range to be honored, presumably by prompting us to set the actual page range value in the print settings.)

Comment 2

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #1)

Note: if you build with bug 1659005's patches, you'll encounter this bug in the preview UI as well

These patches have landed now, so now you can perform the STR without needing to print -- just look at the previewed pages.

(A local workaround is to just adjust the page range after you've changed the printer in step 6 - that gets the page range to be honored, presumably by prompting us to set the actual page range value in the print settings.)

Correcting myself: I was wrong on this workaround -- it's not sufficient to fix things. Really, you have to turn off the page range off & on entirely (Custom -> All, All -> Custom) in order for the page range fields to start to be respected (after you've switched printers).

Comment 4

[Emma Malysz]

The error is from this: https://searchfox.org/mozilla-central/source/toolkit/components/printing/content/print.js#217-219

printAllOrCustomRange is not in this.settings, and so even though the start/end ranges are correct, it won't honor the range.

I'll look more into why that's happening.

Comment 5

[Emma Malysz]

Attachment: Bug 1661852, honor printRange option and update startRange when the number of pages changes

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a7975fa65a25
honor printRange option and update startRange when the number of pages changes r=emalysz

Comment 8

[Mark Striemer [:mstriemer]]

Comment on attachment 9173531 [details]
Bug 1661852, honor printRange option and update startRange when the number of pages changes

Beta/Release Uplift Approval Request

• User impact if declined: Print range changes can result in a blank preview document, or tab crashes on macOS
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See comment 0

Also bug 1662136 comment 0.

• List of other uplifts needed: Bug 1662136
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): Fixes a bug, but also removes an assertion from the c++ code. The assertion caused some crashiness and couldn't hold true in this case though, so likely shouldn't be there
• String changes made/needed: No

Comment 9

[Cristian Brindusan [:cbrindusan]]

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

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9173531 [details]
Bug 1661852, honor printRange option and update startRange when the number of pages changes

Approved for 81.0b6.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/9b81784ca38c

Comment 12

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

Hello,

Cannot properly verify if this issue is fixed or not because of 1663445. Once the Save to PDF buttons is closed, the old modal is displayed and the PDF stored is corrupt. Will CC myself on both issues so I can verify them once 1663445 is fixed.

Comment 13

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

(Uh, I'm not sure what info you want from me on this? 🙂)