1798965 - Simplified PDF print mode (using Reader mode behind the scenes) sometimes causes text to be impossibly small because image widths are unbounded

Status: VERIFIED FIXED

(Toolkit :: Printing, defect, P3)

Description

[Jeff Fortin]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:105.0) Gecko/20100101 Firefox/105.0

Steps to reproduce:

1. Go to this blog post by Google (or this article by the BBC or, to a lesser extent, this article from La Presse for example)
2. Open the print dialog (Ctrl+P)
3. Choose "Save to PDF" as the "Destination"
4. Choose "Simplified" instead of "Original" in the Format setting

Actual results:

Font sizes become extremely small, making it near impossible to read and turning paragraphs into extremely long line lengths which further impairs readability, and is usually not what you want in a PDF archival document (you just want the non-content cruft to be removed, and to have standardized clean readable fonts).

Expected results:

As I suspect this has something to do with image sizes/widths, it should set a reasonable max width (and auto height) for images to fit within the constraints of reasonable readable texts, like the regular Reader mode does. I assume this is based on Reader code because it seems to have been implemented through bug #962433.

Comment 1

[Jeff Fortin]

Attachment: Comparing the "Original" vs "Simplified" formatting in Firefox's native "Save to PDF" print destination

Comment 2

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

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

[Raluca Popovici, Desktop QA]

I was able to reproduce this issue on Ubuntu 18.04 and MacOS 11.6 using Fx 106.0.5, Fx 107.0(beta) and Nightly 108.0a1 testing with the page from the following link https://opensource.googleblog.com/2021/09/using-saliency-in-progressive-jpeg-xl-images.html and with "Simplified" option.

Comment 4

[Daniel Holbert [:dholbert]]

mconley, maybe you could take a look here or know who might be good to point at this, given your involvement in bug 962433?

I don't know precisely how we hook into reader mode for printing, but one interesting observation: if I start out by viewing the article in reader mode and open the print dialog, then everything is fine... I'm not sure if the "activate reader mode, hit print" vs. "print, and choose simplified formatting" flows are meant to behave the same or not, but it seems like maybe they should be expected to do the same thing? So that might be a hint about a possible race condition or unintended behavior difference.

(I also don't know how to activate reader mode for a reduced testcase, which makes it a bit trickier to investigate; if anyone knows how to make the button show up unconditionally, that'd be helpful to note here too.)

Comment 5

[Daniel Holbert [:dholbert]]

Note that on the original Google Blog article as well as Reader Mode itself, the img has max-width: 100%; which is supposed to avoid this issue. In reader mode, that comes from here:

https://searchfox.org/mozilla-central/rev/219df29d0fb5d8928ae41bba4a605046de411cf0/toolkit/themes/shared/aboutReader.css#767-770

.moz-reader-content * {
  max-width: 100%;
  height: auto;
}

So that styesheet must be failing to apply, or somehow not working, when this bug happens.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

I'm actually going to redirect to mstriemer who added the simplified printing capability in bug 1666247 to work with the new print preview dialog.

Comment 7

[Mark Striemer [:mstriemer]]

Did some sleuthing 🕵️‍♂️

I suspect this regressed 5 years ago in bug 1417837. It changed changed the aboutReader.css styles to use classes rather than ids.

So prior to that change, the reader mode styles would've been applying correctly since the original print with reader mode code set #moz-reader-content. Eventually that code was used by the new print dialog, unchanged.

Comment 8

[Mark Striemer [:mstriemer]]

So we should set a class instead of an id in PrintingChild.parseDocument().

Comment 9

[Fred Chasen [:fchasen]]

Attachment: Bug 1798965 - Use classes for simplified print mode generated content r?mstriemer

Updates to using the classes defined in CSS instead of Ids when generating the HTML content for the simplified print mode.

Sets padding to 0 for the body to allow customizing the margins in the print dialog.

Comment 10

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

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:fchasen, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit auto_nag documentation.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/be921a657fa3
Use classes for simplified print mode generated content r=mstriemer

Comment 12

[Serban Stanca [:SerbanS]]

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

Comment 13

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

Since nightly and release are affected, beta will likely be affected too.
For more information, please visit auto_nag documentation.

Comment 14

[Ardelean Oana, Desktop QA]

Reproducible with a 2023-01-09 Nightly build on Windows 10.
Verified as fixed on Firefox 110.0b2(build ID: 20230117185909) and Nightly 111.0a1(build ID: 20230118095131) on Windows 10, macOS 12, Ubuntu 22.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

This grafts cleanly to ESR102 - do you want to nominate it for uplift?

Comment 16

[Fred Chasen [:fchasen]]

Comment on attachment 9305011 [details]
Bug 1798965 - Use classes for simplified print mode generated content r?mstriemer

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Fixes a somewhat common printing issue when large images are included in simplified mode.
• User impact if declined: Some website with extra-large images will have unreadable small text when printed.
• Fix Landed on Version: 110.0b2
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Only affects printing in simplified mode and adjust a few CSS classes.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9305011 [details]
Bug 1798965 - Use classes for simplified print mode generated content r?mstriemer

Approved for 102.8esr.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr102/rev/5057ac4867df

Comment 19

[Ardelean Oana, Desktop QA]

Hello,
This issue is verified as fixed on macOS 12 and Ubuntu 22 on Firefox 102.8.0esr(build ID: 20230206170028) . However, it is still reproducible on Windows 10.
Should I open a new issue for Windows only?

Comment 20

[Fred Chasen [:fchasen]]

Thanks Ardelean, yep if you can create a new issue I'll take a look on Windows.
Do you know if Windows 11 is also affected?

Comment 21

[Ardelean Oana, Desktop QA]

Hello,
I could also reproduce this on Windows 11.
I filled bug 1815833 for the Win-only issue.