1833682 - Fix multiple pages-per-sheet scaling regression

Status: VERIFIED FIXED

(Core :: Printing: Setup, defect)

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: Bug 1833682. Fix n-pages-per-sheet regression, to ensure we properly scale pages beyond the first sheet. r=dholbert,alaskanemily

When splitting out the code for bug 1830900 I missed this rather important line.
That bug changed the code to store certain values on a per-sheet basis, but
forgot to include this line to calculate them for all sheets (as opposed to
just the first sheet).

Comment 2

[Jonathan Watt [:jwatt]]

[Tracking Requested - why for this release]: Printing multiple pages-per-sheet is broken without it.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1830900

Comment 4

[Daniel Holbert [:dholbert]]

Here are some STR here, for reproducing the issue here verifying that this bug is fixed (and reproducing/verifying it when we uplift to 114):

1. Visit some document with many pages, e.g. https://en.wikipedia.org/wiki/Alice%27s_Adventures_in_Wonderland
2. Ctrl+P to open Print Preview
3. Choose "More Settings" and then "Pages Per Sheet: 2"
4. Scroll through the print-previewed document.
5. Now try changing to "Pages per sheet: 4", and scroll through the print-previewed document.

EXPECTED RESULTS:
The correct number of pages per sheet properly shown on all sheets.

ACTUAL RESULTS:
1 page shown on each sheet. (The missing pages are lost entirely)
For the 2-pages-per-sheet scenario, the page runs off the bottom of the sheet.

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: screenshot of the bug

Here's a screenshot of what the issue looks like, with 2 pages per sheet (left) and 4 pages per sheet (right).

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5c3497c1599f
Fix n-pages-per-sheet regression, to ensure we properly scale pages beyond the first sheet. r=dholbert

Comment 7

[Norisz Fay [:noriszfay]]

Backed out for causing assertion failures on PrintedSheetFrame.cpp

Backout link

Push with failures

Failure log

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bf5802708361
Fix n-pages-per-sheet regression, to ensure we properly scale pages beyond the first sheet. r=dholbert

Comment 9

[Cristian Tuns]

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

Comment 10

[Jonathan Watt [:jwatt]]

Comment on attachment 9334642 [details]
Bug 1833682. Fix n-pages-per-sheet regression, to ensure we properly scale pages beyond the first sheet. r=dholbert,alaskanemily

Beta/Release Uplift Approval Request

• User impact if declined: Printing multiple pages-per-sheet is broken after the first sheet.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce: See Daniel's comment 5.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Simple fix.
• String changes made/needed:
• Is Android affected?: No

Comment 11

[Donal Meehan [:dmeehan]]

Comment on attachment 9334642 [details]
Bug 1833682. Fix n-pages-per-sheet regression, to ensure we properly scale pages beyond the first sheet. r=dholbert,alaskanemily

Approved for 114.0b6.

Comment 12

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3b06b9377ea4

Comment 13

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

Reproducible on a 2023-05-17 Nightly build on macOS 12 using the STR from Comment 4.
Verified as fixed on Firefox 114.06(build ID: 20230518180043) and Nightly 115.0a1(build ID: 20230521210743) on macOS 12, Windows 10, Ubuntu 22.
Print Preview now displays the correct number of pages per sheet.

Comment 14

[Acasandrei Beatrice (needinfo me)]

Perfherder has detected a build_metrics performance regression from push 88f507752a8b57949b64bd4be8717dab605c9cad. Could you confirm if the backout for this patch has caused this regression alert summary ?