1577711 - Text disappears when changing page layout on html files with multiple frames

Status: VERIFIED FIXED

(Core :: Print Preview, defect, P2)

Description

[Peter Magyari (Desktop QA)]

[Affected versions]
Firefox Nightly 70.0a1

[Affected platforms]
Windows 7/10 (64-bit)
Ubuntu 18.04

[Steps to reproduce]

1. Launch Firefox Nightly
2. Download html file from https://drive.google.com/file/d/0BzOV3ybvBASXVlV1aE5ZMmRkYzQ/view
3. Open the printing_vertical_frames.html in a new window
4. Open the hamburger-menu and click on Print
5. In print-preview mode change layout to landscape

[Expected result]
Page should be set in landscape and the text should be visible.

[Actual Result]
The text disappears.

[Regression-Range]
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=637f32b7e43c06e66c53df5824352c5e8e946d11&tochange=f74c9d2552939875df983d9d634ee4fb1fea42f9

Bug 1568055 seems to be the culprit.
Nika, could you take a look?

Comment 1

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

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 2

[Daniel Holbert [:dholbert]]

Thanks for the report and the regression range!

[Tracking Requested - why for this release]: Regression in Nightly 70 that we should strive to fix before it hits release.

Comment 3

[Daniel Holbert [:dholbert]]

I can reproduce locally in current Nightly, BTW.

I also tried a debug build, and got two interesting assertions:
(1) when I change layout to landscape (STR step 5), I get a non-fatal assertion failure:
[Child 1659, Main Thread] ###!!! ASSERTION: There must always be an XMost PO!: 'smallestPO', file layout/printing/nsPrintJob.cpp, line 1563

(2) When I change layout *back to portrait (not in STR but interesting to do), I get a fatal assertion failure:
Assertion failure: false (MOZ_ASSERT_UNREACHABLE: Init failed?), at layout/printing/nsPrintJob.cpp:474

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: (testcase helper file #1)

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: (testcase helper file #2)

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: testcase 1 (from reporter)

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: testcase 2 (reduced, standalone)

Comment 8

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1577711 - Part 1: Revert calling SetDocument earlier during clone from 1568055,

This change was causing issues with images not displaying and iframes not
displaying correctly after multiple print preview calls.

Comment 9

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1577711 - Part 2: Perform frame static clone after parent static clone,

This is done by delaying the code within nsFrameLoader::CreateStaticClone until
after the document has been created. The nsFrameLoader is re-discovered using
the subframe BrowsingContext's mEmbedderElement.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1d4cb1ea199e
Part 1: Revert calling SetDocument earlier during clone from 1568055, r=smaug
https://hg.mozilla.org/integration/autoland/rev/f032a0bd2725
Part 2: Perform frame static clone after parent static clone, r=smaug

Comment 11

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/1d4cb1ea199e
https://hg.mozilla.org/mozilla-central/rev/f032a0bd2725

Comment 12

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 9090254 [details]
Bug 1577711 - Part 1: Revert calling SetDocument earlier during clone from 1568055,

Beta/Release Uplift Approval Request

• User impact if declined: iframes and images may not behave correctly when printing an in print preview
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: QA found both related bugs in bug 1577597 and bug 1577711, and attempting the STRs from those two bugs would be excellent.
• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): The affected code is poorly covered by automated tests, which is how this bug made it into beta in the first place. It may be possible to introduce another regression with this follow-up.
• String changes made/needed: none

Comment 13

[Peter Magyari (Desktop QA)]

Verified fixed on Windows 7, Windows 10 and Ubuntu 18.04 using Firefox Nightly 71.0a1 (20190908214439)
I've also verified Bug 1577597 which has also been fixed by the same update.

Comment 14

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9090254 [details]
Bug 1577711 - Part 1: Revert calling SetDocument earlier during clone from 1568055,

Fix verified in nightly, let's risk it for beta 6 since I don't want to ship this regression to print preview/printing.

Comment 15

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/468cfd64d103
https://hg.mozilla.org/releases/mozilla-beta/rev/7ba2b66f0e32

Comment 16

[Peter Magyari (Desktop QA)]

Verified fixed on Windows 7, Windows 10 and Ubuntu 18.04 using Firefox 70.0b6(20190912160217)