1714513 - Video suggestions from youtube are shown out of page during print preview

Status: VERIFIED FIXED

(Core :: Print Preview, defect)

Description

[Bogdan Maris, Desktop QA]

Attachment: Image showing the issue

Affected versions

• Firefox 90.0b3
• Latest Nightly 91.0a1

Affected platforms

• Windows 10
• Ubuntu 18.04
• MacOS 11.3

Steps to reproduce

1. Start Firefox with a new profile
2. Visit youtube.com
3. Print (ctrl+P or from hamburger etc)
4. Have Save to PDF option in Destination (to make it easier to save, I don't have a printer set up to my PC).
5. Save the pdf locally

Expected result

• The page shown in preview matches the actual printed page.

Actual result

• The video suggestions from youtube are out of bounds in print preview. The page is correctly printed though in some cases.
• After saving the file on disk, the message Print preview error, An error occurred while printing is displayed but only on Windows 10 and Ubuntu 18. (I also tested on another w10 machine and the error was not shown so I'm not sure why that is displayed)
• This is the message from Browser console when the error hits: [Exception... "Failure" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: resource://gre/modules/Prompter.jsm :: openPromptSync :: line 1116" data: no]

Regression range 1

• Out of bounds video suggestions range
• First bad: 029a5e8fa8f3179b63a7bf291ea689c4dd642492
• Last good: 099906bf5f7436a72cee78b61075574bf073c0e6
• Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=099906bf5f7436a72cee78b61075574bf073c0e6&tochange=029a5e8fa8f3179b63a7bf291ea689c4dd642492
• Potential regressor: Bug 1700379 - Hit testing on fission iframes in print preview is messed up. This is a duplicate of Bug 1710059 - [fission] Hit testing scaled remote iframes is broken.

Regression range 2

• Error message range
• First bad: 411d3031b1b4c0f8b9ff3009ceb21ba4cfdc601e
• Last good: d7a58cff3e920dfbfdfe29c76a2822f7b916c0d3
• Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=d7a58cff3e920dfbfdfe29c76a2822f7b916c0d3&tochange=411d3031b1b4c0f8b9ff3009ceb21ba4cfdc601e
• Potential regressor: Bug 1540737 Add layerless painting

Additional notes

• Image showing the issue is attached.
• When the error message is thrown the PDF is corrupt and can't be opened in any browser or 3rd party app I tried. (Adobe, Foxit)

Suggested severity

• Not sure how many websites this could affect but I would make it an S3 for now. Please fiddle with the severity if anyone thinks this could be more serious.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

The error message thing looks fairly unlikely. But yeah it seems I introduced a clipping regression with my refactoring, will address.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1714513 - Use ClipContentDescendants rather than ClipContainingBlockDescendants in nsPageContentFrame::BuildDisplayList. r=miko,mattwoodrow

That way we guarantee to clip abspos boxes etc. This used to work
because this was further up the stack in nsPageFrame rather than
nsPageContentFrame:

https://hg.mozilla.org/integration/autoland/rev/029a5e8fa8f3179b63a7bf291ea689c4dd642492#l6.262

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9225223 [details]
Bug 1714513 - Use ClipContentDescendants rather than ClipContainingBlockDescendants in nsPageContentFrame::BuildDisplayList. r=miko,mattwoodrow

Beta/Release Uplift Approval Request

• User impact if declined: comment 0
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: comment 0
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): One-liner to clip a bit more agressively to the page content size.
• String changes made/needed: none

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/884543a0d0ae
Use ClipContentDescendants rather than ClipContainingBlockDescendants in nsPageContentFrame::BuildDisplayList. r=mattwoodrow

Comment 5

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

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

Comment 6

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/884543a0d0ae

Comment 7

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

Upstream PR merged by moz-wptsync-bot

Comment 8

[Bogdan Maris, Desktop QA]

Verified that using the Nightly after the fix landed, the page looks good in print preview, but the error is still there when printing.
Based on the info from comment 0 (Regression range 2) this might have been something from bug 1540737, Matt do you want me to log a new bug for that?

Comment 9

[Julien Cristau [:jcristau]]

Comment on attachment 9225223 [details]
Bug 1714513 - Use ClipContentDescendants rather than ClipContainingBlockDescendants in nsPageContentFrame::BuildDisplayList. r=miko,mattwoodrow

regression fix, approved for 90.0b6

Comment 10

[Bogdan Maris, Desktop QA]

(In reply to Bogdan Maris [:bogdan_maris], Release Desktop QA from comment #8)

Verified that using the Nightly after the fix landed, the page looks good in print preview, but the error is still there when printing.
Based on the info from comment 0 (Regression range 2) this might have been something from bug 1540737, Matt do you want me to log a new bug for that?

I logged a new bug for the error (bug 1715771), will leave this one only for the print preview display of the page.

Comment 11

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/65c41f56b54b

Comment 12

[Bogdan Maris, Desktop QA]

Also verified that the page looks good in print preview using Firefox 90.0b6 across platforms (Windows 10 64bit, macOS 11.3 and Ubuntu 18.04).