1747997 - Print preview of pages without a title does not match the Save-to-PDF output

Status: NEW

(Core :: Print Preview, defect)

Description

[Monica Chiorean]

Attachment: Different print preview.png

Affected versions

• Beta 96.0b10 (64-bit)
• Nightly 97.0a1(20211230095735)
• Release 95.0.2 (20211218203254)

Affected platforms

• Win 10 x64
• Win 7 x86
• macOS 10.13
• Ubuntu 20.04 x64

Steps to reproduce

1. Open Mozilla FF.
2. Open the following link https://bug1722890.bmoattachments.org/attachment.cgi?id=9234328.
3. From Hamburger menu select Print.
4. On print preview select “Save to pdf”.
5. Compare the print preview and the pdf file.

Expected result
Same file should be displayed on both sides.
Actual result
Print preview is different from the pdf file, because on the print preview on the upper left side there is a link, while on the pdf file it is displayed Firefox text(check screenshot attached).

Suggested Severity

• S4, since it is a display issue only this type of links.
  Issue is not reproducing on: OLD->Nightly build from 4th of May.

Regression range

• pushlog_url: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7dca70384eb6dbbf392d942f51e66f3fe091c950&tochange=532840a9ae885348268858505f77fbac21e50caf

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Core::Print Preview' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=36a1cc27b2645047fd2511b15ecf28a2838e3769&tochange=532840a9ae885348268858505f77fbac21e50caf

Comment 3

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

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

Comment 4

[Marco Castelluccio [:marco]]

Mark, can you take a look?

Comment 5

[Daniel Holbert [:dholbert]]

This reproduces on any page without a <title>, I think.

It reproduces on this extremely-reduced testcase:

data:text/html,Hello

...but not on:

data:text/html,<title>TITLE</title>Hello

(For the former data URI here, print preview shows the URL at the top-left, whereas the printout shows the text "Nightly".)

Comment 6

[Daniel Holbert [:dholbert]]

(Given the regression range, I suspect this is a bug in the JS code that sets up the print-preview visualization, rather than in the platform code to support printing/print-preview. Given that, I'm reclassifying as Toolkit|Printing.)

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

Comment 8

[Daniel Holbert [:dholbert]]

Poking around in gdb, it looks like we're asked to draw &T (title) for the left-header and &U (URL) for the right-header.

We expand &T to mPD->mDocTitle here, but unfortunately mPD->mDocTitle actually contains the page URL in this case.

mPD->mDocTitle is populated a bit earlier, and it looks like we use a different fallback flag for the print-preview vs. printing cases.

In nsPrintJob::SetupToPrintContent, we have:

  nsAutoString docTitleStr;
  nsAutoString docURLStr;
  GetDisplayTitleAndURL(
      *printData->mPrintObject->mDocument, printData->mPrintSettings,
      DocTitleDefault::eDocURLElseFallback, docTitleStr, docURLStr);

In nsPrintJob::DoPrint, we have:

    nsAutoString docTitleStr;
    nsAutoString docURLStr;
    GetDisplayTitleAndURL(*aPO->mDocument, mPrt->mPrintSettings,
                          DocTitleDefault::eFallback, docTitleStr, docURLStr);

The eDocURLElseFallback vs. eFallback seems to be the relevant thing here, with the former causing us to fall back to the URL, and the latter causing us to fall back to "Nightly" (or "Firefox") instead. And importantly, nsPrintJob::DoPrint is only executed for "actual" print operations (including print-to-PDF), at least in current builds. So that's why we're not getting a URL at the top-left in those actual print operations.

Comment 9

[Daniel Holbert [:dholbert]]

These GetDisplayTitleAndURL calls (with the disagreeing fallback flags) come most-recently from jwatt in bug 1619403, though the discrepancy predated that commit with older enum names -- they used to be eDocTitleDefURLDoc vs eDocTitleDefBlank:
https://hg.mozilla.org/mozilla-central/rev/885a99d3d16c#l1.210

So there's been some disagreement between these two code-paths for quite a while - it goes back at least to 2006 in bug 361844, with these two lines:

 GetDisplayTitleAndURL(mPrt->mPrintObject, &docTitleStr, &docURLStr, eDocTitleDefURLDoc); 

vs.

GetDisplayTitleAndURL(aPO, &docTitleStr, &docURLStr, eDocTitleDefBlank); 

https://searchfox.org/mozilla-central/diff/01ccd95a112c5e9e5c64a66260e645839875fb21/layout/printing/nsPrintEngine.cpp#1956
https://searchfox.org/mozilla-central/diff/01ccd95a112c5e9e5c64a66260e645839875fb21/layout/printing/nsPrintEngine.cpp#2397

I don't immediately see why this only would have started causing trouble recently (maybe DoPrint was being called even for print-preview in some cases, and now it's not?)

jwatt, do you know why we make different choices for fallback enum in these two different code-paths? Given your refactoring in bug 1619403, I'm wondering if you have any insight into what's going on here.

Comment 10

[Daniel Holbert [:dholbert]]

And this probably does belong in Core|Print-Preview after all, given that the differing behavior seems to be hardcoded into the C++ code. (Even though it was somehow triggered by a non-C++ change, based on the regression analysis, which is... odd.)

--> Reclassifying back.

Comment 11

[Daniel Holbert [:dholbert]]

I tested this in an old build just before bug 1666247's patches, to see why this "regressed" there.

It looks like the relevant difference is: before bug 1666247, when SetupToPrintContent calls GetDisplayTitleAndURL for print-preview, it finds that aSettings->GetDocURL() is empty; and that causes GetDisplayTitleAndURL to fall back to the branded name for its aTitle outparam, even though it was passed the fallback-to-URL enum eDocURLElseFallback. (Ironically, GetDisplayTitleAndURL then just proceeds to figure out the URL for itself, to populate its aURLStr outparam; it doesn't use that dynamically-determined page-URL as the fallback title.)

Whereas after bug 1666247, aSettings->GetDocURL() is nonempty -- it's got the document URL already, presumably provided by JS via one of the changes in bug 1666247 -- and that causes GetDisplayTitleAndURL to be able to fall back to that as the page-title (when given eDocURLElseFallback).

So, tl;dr: bug 1666247 probably caused this visible behavior-change by virtue of helpfully providing the page URI in the settings object, which inadvertently exposed the fact that our missing-page-title fallback behavior is somewhat dependent on the presence/absence of that URI, specifically in the codepath that we take for print preview.

So, two takeaways:
(1) It seems odd/cobwebby that we have these two different fallback modes, and we use one for print-preview (and for early stages of actual printing), vs. the other for the final stage of actual-printing. Perhaps we should drop eDocURLElseFallback, and/or make our usage consistent between the two callsites?

(2) If needed/simpler, we could also probably massage the JS to avoid providing the page's URL to the aSettings object, to restore our old behavior without needing to mess with the C++ here. (But ideally it would be good to have a coherent story around how the underlying codepaths behave, too.)

[I'm curious for jwatt's thoughts on both of these points; leaving ni open for his feedback on these when he's around.]

Comment 12

[Ina Popescu, Desktop QA]

Same issue is encountered on following links:

https://bug1715803.bmoattachments.org/attachment.cgi?id=9226355
https://bug1722457.bmoattachments.org/attachment.cgi?id=9233262
https://bug1722188.bmoattachments.org/attachment.cgi?id=9233776

This was tested on Firefox 110.0b7.

Comment 13

[Daniel Holbert [:dholbert]]

(In reply to Ina Popescu from comment #12)

Same issue is encountered on following links:

Yup -- per comment 5, any page without a <title> is affected. (And the links you provided do indeed lack a <title>.)

Comment 14

[Jonathan Watt [:jwatt]]

(In reply to Daniel Holbert [:dholbert] from comment #11)

[I'm curious for jwatt's thoughts on both of these points; leaving ni open for his feedback on these when he's around.]

I agree with both points, FWIW.