1917715 - Print preview doesn't work properly for some SVG unless "print backgrounds" is checked

Status: VERIFIED FIXED

(Core :: Print Preview, defect)

Description

[Mike Hommey [:glandium]]

Attachment: testcase.html

The attached HTML is reduced from an HTML file containing many things, including a full QR code as an embedded SVG image with more M instructions in the <path>. Print preview shows a big black box instead of the SVG as it can be seen on the page, unless "print backgrounds" is checked.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: testcase 2 (reduced)

It looks like we're rendering

  <rect x="10" y="10" width="80" height="80" fill="white"></rect>

as having a black fill-color, for some reason (only when "Print Backgrounds" is un-checked).

Here's a testcase. Expected rendering (for a color printer target like our "Save to PDF" print backend) is a lime circle in a cyan square. Actual rendering is that plus an unwanted black square.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Seems like a regression..

Comment 3

[Daniel Holbert [:dholbert]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)

Seems like a regression..

Indeed. For some reason there are a year or so worth of mozregression builds that aren't launching at all for me for some reason right now (no Firefox window appears at all), but I narrowed it to this still-quite-wide window:
Good: 2021-09-01
Bad: 2023-04-21

Also: from local testing, I think this is specific to rect which makes me suspect it's a bug in the rect-specific WebRender path builder, like bug 1892786 is. Per bug 1892786 comment 35, we have a special codepath for <rect>-with-fill-set-to-a-color, which runs some utility code to convert the specified fill to a color that WebRender can understand. Maybe that's broken in this case for some reason (e.g. because we're compositing over a document with no background, or something).

Comment 4

[Emilio Cobos Álvarez [:emilio]]

Mozregression says: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=89b0f422a716f208ec0e0e85aaa5ac7c0b759ee9&tochange=0a4cbd771ad2b7b71032136e2b7375e0834ef9f7

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Bug 1686654?

Comment 6

[Emilio Cobos Álvarez [:emilio]]

confirmed by flipping the pref from bug 1686654.

Comment 7

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

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

:nical, since you are the author of the regressor, bug 1686654, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 8

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1917715 - Don't use nsLayoutUtils::GetColor for SVG rects. r=nical,dholbert

It's intended for text colors only (and it applies auto-darkening in
printing), see nsLayoutUtils::DarkenColorIfNeeded.

Comment 9

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1917715 - Rename nsLayoutUtils::GetColor. r=#layout

It's suitable for foreground colors only.

Comment 10

[Daniel Holbert [:dholbert]]

no need to needinfo nical; emilio's already taken this.

Comment 11

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1917715 - Add a test for this bug. r=dholbert,nical

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/465db4e0254f
Don't use nsLayoutUtils::GetColor for SVG rects. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/8065270635ad
Rename nsLayoutUtils::GetColor. r=longsonr,dholbert

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a9997ea14b01
Add a test for this bug. r=dholbert

Comment 14

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/465db4e0254f
https://hg.mozilla.org/mozilla-central/rev/8065270635ad

Comment 15

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

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

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

For more information, please visit BugBot documentation.

Comment 16

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 9423696 [details]
Bug 1917715 - Don't use nsLayoutUtils::GetColor for SVG rects. r=nical,dholbert

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?: Yes
• 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): It's a long-standing regression, but the patch is rather trivial.
• String changes made/needed: none
• Is Android affected?: Yes

Comment 17

[Dianna Smith [:diannaS]]

Comment on attachment 9423696 [details]
Bug 1917715 - Don't use nsLayoutUtils::GetColor for SVG rects. r=nical,dholbert

Approved for 131.0b5

Comment 18

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/e536bd704540

Comment 19

[Norisz Fay [:noriszfay]]

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

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9423696 [details]
Bug 1917715 - Don't use nsLayoutUtils::GetColor for SVG rects. r=nical,dholbert

Approved for 128.3esr. I'm also taking the second patch for easier ESR128 uplift ergonomics down the road.

Comment 21

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/6406a33feba2
https://hg.mozilla.org/releases/mozilla-esr128/rev/9677c428f761
https://hg.mozilla.org/releases/mozilla-esr128/rev/91f6011b864e

Comment 22

[Raluca Popovici, Desktop QA]

Reproduced the issue on Windows 10x64 on Fx 130.0 by using the provided testcase.html from comment 0.
Verified as fixed for testcase.hmtl from comment 0 on Windows 10x64 and MacOS 11.7.4 using Fx 131.0b5 and Nightly 132.0a1( 20240911).

Comment 23

[Raluca Popovici, Desktop QA]

Verified as fixed for testcase.hmtl from comment 0 on Windows 10x64 using Fx 128esr(treeherder build).