1699837 - Remote frames are zoomed in or out when printing to PDF

Status: RESOLVED FIXED

(Core :: Printing: Output, defect, P3)

Description

[Julian Descottes [:jdescottes]]

I mentioned this in Bug 1695806, but I think it's worth filing a separate bug to make sure this gets properly triaged.

STRs:

• enable Fission
• open a page with remote frames (eg http://janodvarko.cz/tests/fission/case10/index.html)
• print > save to PDF

ER: The content of the remote frame should look the same as without fission
AR: The content of the remote frame is zoomed out

Comment 1

[Julian Descottes [:jdescottes]]

Attachment: bug_1699837_without_fission.pdf

Comment 2

[Julian Descottes [:jdescottes]]

Attachment: bug_1699837_with_fission.pdf

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

When I print the same page as PDF with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:88.0) Gecko/20100101 Firefox/88.0 ID:20210312033752 the remote frames content is zoomed in. So maybe it's system / platform dependent what actually happens?

Comment 4

[Emilio Cobos Álvarez (:emilio)]

So I dug into this a bit and I'm pretty sure that this is due to the various page scales etc not applying to the remote iframe. So somewhat a Graphics issue.

This is very visible on a test-case like this. The red area should always be half of the content area, but it is scaled up because we don't seem to account for the page scale transform that shrinks the iframe.

Tim, do you happen to know how do we compute effective scales for remote content like this?

Comment 5

[Timothy Nikkel (:tnikkel)]

For non-wr I think we send the scale to the child here

https://searchfox.org/mozilla-central/rev/36f79bed679ad7ec46f7cd05868a8f6dc823e1be/layout/generic/nsSubDocumentFrame.cpp#1290

and we use that value in the child here

https://searchfox.org/mozilla-central/rev/36f79bed679ad7ec46f7cd05868a8f6dc823e1be/layout/painting/nsDisplayList.cpp#2326

For wr I'm not sure how it works, the corresponding wr call to AddEffectUpdate just sends a unit scale. So maybe when wr rasterizes in the gpu/parent proc it assembles the wr display list and has all the info it needs that way?

Comment 6

[Jim Mathies [:jimm]]

Looks like layout folks have this one.

Comment 7

[Neha Kochar [:neha]]

Emilio, gentle reminder about this M7a issue assigned to you. :)

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Yup, I was blocked on some snapping issues, but bug 1709062 allows me to dig into them and fix them.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1699837 - Make sure that remote iframes honor print settings. r=mattwoodrow

This fixes it since we honor the print resolution properly now. However
print setting updates are still broken.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1699837 - Invalidate style for visible area changes in subdocuments. r=mattwoodrow

The subdocuments do use it. Without fission, we don't have dynamic
visible area changes, but with the incoming patches we will.

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1699837 - Use a few more typed units in CrossProcessPaint. r=mattwoodrow

This is just cleanup while I was looking at why the test still fails
sometimes locally even with my patch. It is a bit racy because
BrowserChild's visible rect is not set by the time we do the
cross-process paint, so we clip everything...

This doesn't affect users and I have some upcoming PTO though, so will
look into it when I'm back.

Depends on D115263

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6df4f3b1c288
Invalidate style for visible area changes in subdocuments. r=mattwoodrow

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eb21f0b64f03
Use a few more typed units in CrossProcessPaint. r=mattwoodrow

Comment 14

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/6df4f3b1c288
https://hg.mozilla.org/mozilla-central/rev/eb21f0b64f03

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/43a82597dade
Make sure that remote iframes honor print settings. r=mattwoodrow

Comment 16

[Dorel Luca [:dluca]]

Backed out changeset 43a82597dade (bug 1699837) for Crashtest in layout/printing/crashtests/1671503.html. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer?job_id=342518659&repo=autoland&lineNumber=17734

Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=43a82597dadedca7e887d61bda2610648c0de9b9

Backout:
https://hg.mozilla.org/integration/autoland/rev/4a5922c63344d8777360ecc1bcc0e1f078f1986b

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/30c1f5e41d17
Make sure that remote iframes honor print settings. r=mattwoodrow

Comment 18

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/30c1f5e41d17

Comment 19

[[:fabrice] Fabrice Desré]

Hi Emilio,

This patch breaks builds that have printing disabled, since BrowserChild.cpp now includes nsDeviceContextSpecProxy.h which itself includes mozilla/layout/printing/DrawEventRecorder.h that is not included because of https://searchfox.org/mozilla-central/rev/077501b34cca91763ae04f4633a42fddd919fdbd/layout/moz.build#31

What is the best way forward here? Should we consider that it's not possible anymore to disable the printing support and recognize that clearly by removing --disable-printing, or is there a sane way to #ifdef our way around this patch?

Comment 20

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1699837 - Follow-up: Fix --disable-printing builds.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/57328f12e67a
Follow-up: Fix --disable-printing builds.

Comment 22

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/57328f12e67a