1654489 - print preview failure when huge margins are specified

Status: VERIFIED FIXED

(Core :: Print Preview, defect)

Description

[Mahyar]

Attachment: Capture.PNG

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/84.0.4147.89 Safari/537.36

Steps to reproduce:

Menu => Print => Page Setup => Margins & Header/Footer =>
Change the margin to 20 or higher number

Actual results:

The print Preview popup with "Progress: Preparing ..." was opened and not closed.
After closing and opening the browser, the issue remains.

Comment 1

[Alice0775 White]

Mahyar,
Could you please attach results of about:support (Help –> Troubleshooting Information)?

Comment 2

[Alin Ilea, Desktop QA]

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Firefox/78.0

Hi,

I have managed to reproduce this issue on release version 78.0.2, Beta 79 and latest Nightly 80.0a1 (2020-07-27) using Windows 10.
Further, I will move this over to a component so developers can take a look over it. If this is not the correct component please feel free to change it to an appropriate one.
I attached here my about:support from latest Nightly build.

Thanks for the report.

Comment 3

[Alin Ilea, Desktop QA]

Attachment: aboutsupport.txt

Comment 4

[Jonathan Kew [:jfkthame]]

It sounds like the issue is margins being set so large that no actual content would be able to appear on the page?

I can't seem to reproduce this locally; even when I set 20" margins, the print preview appears (with a large number of pages, empty except for header and footer -- not very useful, but it seems like the expected outcome).

Maybe the exact behavior is dependent on the printer driver in some way, or other settings? Or is it specific to the webpage being printed/previewed?

Comment 5

[Emilio Cobos Álvarez (:emilio)]

I couldn't reproduce either on any of my two windows machines. Alin, could you give us a recording of the issue so that we can check if we've missed something trying to repro it? Thanks a lot.

Comment 6

[Alin Ilea, Desktop QA]

Attachment: 2020-07-29_11h50_44.mp4

Sure, I attached here the screen recording, but I need to mention that the issue seems to not be reproducible all the time, I managed to reproduce it 1 of 3 tries on the same machine with new clean profile each time.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Thanks! I could repro with those margins on that page. Here's a profile: https://share.firefox.dev/2X9aK3D

It seems we either get stuck at layout without being able to make progress, or we make such little progress that it takes forever to finish laying out the page.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1654489 - Properly guard against huge default margins in nsPageFrame::Reflow. r=jfkthame,dholbert

We were already guarding against huge @page { margins } and so on, but
not about huge default margins.

This uses a lambda so that in the case of oversized margins and such we
still show an (empty) page with headers and footers (the existing code
would just make the page zero-size).

I could move the lambda to a separate method if you want or what not,
might be cleaner.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a9309860b5e3
Properly guard against huge default margins in nsPageFrame::Reflow. r=jfkthame

Comment 10

[Alexandru Michis [:malexandru]]

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

Comment 11

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

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9166804 [details]
Bug 1654489 - Properly guard against huge default margins in nsPageFrame::Reflow. r=jfkthame,dholbert

Beta/Release Uplift Approval Request

• User impact if declined: hangs if too-big default margins are specified in the print settings.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: comment 0 / comment 6
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Bails out on a condition that causes us to reflow infinitely because there's not any available space.
• String changes made/needed: none

Comment 13

[Alin Ilea, Desktop QA]

Verified - Fixed in latest Nightly 81.0a1 (20200803213927) using Windows 10.

Comment 14

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 9166804 [details]
Bug 1654489 - Properly guard against huge default margins in nsPageFrame::Reflow. r=jfkthame,dholbert

approved for 80.0b4

Comment 15

[Julien Cristau [:jcristau] (back April 22)]

https://hg.mozilla.org/releases/mozilla-beta/rev/382e9f840ba7

Comment 16

[Julien Cristau [:jcristau] (back April 22)]

Seeing reftest failures on beta, it's quite possible I messed up the rebase there.

https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&group_state=expanded&selectedTaskRun=LytLgm_lSEqbDNRq3_D_HA.0&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&revision=49af6aad45d99cee2de4c13b7b3d557e62406a02&searchStr=reftest

Comment 17

[Julien Cristau [:jcristau] (back April 22)]

Backed out for now:
https://hg.mozilla.org/releases/mozilla-beta/rev/2f7d9a147be6

Comment 18

[Emilio Cobos Álvarez (:emilio)]

Attachment: Simpler patch for beta.

This is easier than doing merge conflicts, and it's the same fix, just without the additional cleanup :)

Comment 19

[Julien Cristau [:jcristau] (back April 22)]

https://hg.mozilla.org/releases/mozilla-beta/rev/08caecf0bdc8

Comment 20

[Julien Cristau [:jcristau] (back April 22)]

Thanks!

Comment 21

[Alin Ilea, Desktop QA]

Verified - Fixed in latest Beta 80.0b5 (build id: 20200806203447) using Windows 10.