1911948 - Current tab crashes when printing an iframe with "break-inside: avoid"

Status: RESOLVED FIXED

(Core :: Print Preview, defect)

Description

[Troy Spradling]

Attachment: index.html

Steps to reproduce:

Printing an iframe with sections using break-inside: avoid. Commenting/uncommenting the last paragraph seems to enable/disable the issue, so it appears to be related to the view port dimensions and width of content.

This was first identified using https://github.com/futurepress/epub.js to view epubs, so the page width and column widths are set in a similar fashion. The iframe width is set to 1942px, the iframe body is 1942 / 2 = 971px (two pages), with column-width being 971 / 2 = 405.5px. I'm not sure if this matters.

I've attached an html test file to demonstrate.

I used mozregression to narrow this down to 118, and before this release it just previews a blank page (a whole other issue). I've tested this on Fedora 40 and MacOS 14.4.1.

Actual results:

The print window loads, but the preview never renders and is stuck "Preparing Preview". There is no output when saving as PDF, and this ends up crashing the current tab.

Expected results:

Current tab doesn't crash, print preview renders, and I'm able to print what's visible in the viewport.

Comment 1

[Troy Spradling]

I've also tested & verified this on release 128, and on nightly as of August 6th, 2024.

Comment 2

[BugBot [:suhaib / :marco]]

The Bugbug bot thinks this bug should belong to the 'Core::Printing: Output' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 3

[Jonathan Kew [:jfkthame]]

I didn't see a crash (though maybe I just didn't wait long enough....), but the issue of being stuck at "Preparing Preview" reproduces for me (on macOS) going back considerably before v.118; mozregression gave me this range, which suggests it regressed in 115:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ffee8e3a7870875ab78af162638548f72cdabddf&tochange=375c5940c253b38f65792c847de04bc50b990b22

Comment 4

[Troy Spradling]

I think this is related: https://bugzilla.mozilla.org/show_bug.cgi?id=1831829

Comment 5

[Jonathan Kew [:jfkthame]]

Yeah, from the range in comment 3, that looks like the most plausible candidate.

I don't think we have the ability to paginate a multicol-inside-an-<iframe> nicely (prior to the regression, the preview shows the entire iframe being scaled to fit within the page), so at least until that is handled, we should exclude this case from being considered as "paginated" here.

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1911948 - Don't let top-level multicol be paginated if it is inside a subdocument. r=tylin

Comment 7

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1911948 - Add the testcase to WPT. r=tylin

Without the patch, this fails with a timeout when it attempts to print.

Comment 8

[BugBot [:suhaib / :marco]]

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

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4ef68788e380
Don't let top-level multicol be paginated if it is inside a subdocument. r=TYLin,emilio
https://hg.mozilla.org/integration/autoland/rev/42dd1bd7ae7e
Add the testcase to WPT. r=TYLin

Comment 10

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/47532 for changes under testing/web-platform/tests

Comment 11

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/4ef68788e380
https://hg.mozilla.org/mozilla-central/rev/42dd1bd7ae7e

Comment 12

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 13

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected.
:jfkthame, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox130 to wontfix.

For more information, please visit BugBot documentation.

Comment 14

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1911948 - Don't let top-level multicol be paginated if it is inside a subdocument.

Original Revision: https://phabricator.services.mozilla.com/D218823

Comment 15

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1911948 - Add the testcase to WPT.

Without the patch, this fails with a timeout when it attempts to print.

Original Revision: https://phabricator.services.mozilla.com/D218825

Comment 16

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: Possible infinite-loop (hang)/crash when attempting to preview/print
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: n/a
• Risk associated with taking this patch: minimal
• Explanation of risk level: Patch avoids the issue by narrowing the scope of the regressing change to exclude iframe'd subdocuments, so reverting to previous behavior for this case
• String changes made/needed: none
• Is Android affected?: yes

Comment 17

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/88c0b256fd0b

Comment 18

[Ryan VanderMeulen [:RyanVM]]

This grafts cleanly to ESR128. Did you want to nominate for uplift there also?

Comment 19

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/0d78bd7b969c

Comment 20

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1911948 - Don't let top-level multicol be paginated if it is inside a subdocument. r=tylin

Original Revision: https://phabricator.services.mozilla.com/D218823

Comment 21

[Phabricator Automation]

esr128 Uplift Approval Request

• User impact if declined: Possible infinite-loop (hang)/crash when attempting to preview/print
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: n/a
• Risk associated with taking this patch: minimal
• Explanation of risk level: just avoids trying to handle pagination of iframe'd documents, which led to the problem case
• String changes made/needed: none
• Is Android affected?: yes

Comment 22

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/d6d5e7b9b336

Comment 23

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/7d5158c7c58b