1895872 - Firefox creates broken PDF when destination internal link missing

Status: VERIFIED FIXED

(Core :: Printing: Output, defect)

Description

[[e-mail]]

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

Steps to reproduce:

1. Open https://yoast.com/internal-linking-for-seo-why-and-how/
2. Print to PDF

Actual results:

An incomplete PDF file is created that will not open due to missing trailer dictionary.

Expected results:

A valid PDF file is created.

Comment 1

[[e-mail]]

After https://bugzilla.mozilla.org/show_bug.cgi?id=1729276 was fixed I tested print to PDF in Firefox nightly on a few random websites. One of them resulted in an invalid PDF file.

Setting the "CAIRO_DEBUG_TAG=1" env var resulted in the following debug information:

TAG ERROR: Link to dest="yoast-seo-shopify" not found

There are two issues here:

1. Firefox is creating a link in the cairo PDF surface without a corresponding destination.
2. Firefox is not checking cairo_surface_status() after cairo_surface_finish() to get the final status and reporting any error state to the user. The user should be notified that the PDF failed to be generated otherwise they may save to PDF expecting to be able to open the PDF file at a later date.

Ideally Firefox should check that it has a valid link destination before creating a link in cairo. If this is not possible I could create a work around in cairo to set the link destination to the same page as the link. Cairo needs to set the link destination to something as a link has already been written to the PDF file with a placeholder object ID. An object with this ID needs to be created in the PDF with a link destination otherwise PDF readers may complain of a missing object.

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

[Jonathan Kew [:jfkthame]]

Thanks for the prompt report, Adrian.

Looking at View Source for the https://yoast.com/internal-linking-for-seo-why-and-how/ page, it appears there is a <section> element with id="yoast-seo-shopify", but it's within a tabbed interface element where it isn't visible by default, so it doesn't make it into the PDF output and we don't see it as a potential destination.

Offhand it seems like it's going to be difficult for Firefox to check the destination before generating a link, unless we do an entire "pre-flight" pass over the document to find out exactly what's going to be rendered. (Also, what if the user has requested only a subset of pages, and the destination is on a page that isn't going to be included?)

If cairo can be made to safely handle the case of a link whose destination doesn't exist, I think that would be much preferable. Is that something you can look into in the near future (so we can avoid shipping with this brokenness)? Thanks!

(For what it's worth, on macOS, where we generate PDF through the Quartz backend, this doesn't seem to be an issue; the link just gets generated with a missing destination. Opening the resulting PDF in both Preview.app and Adobe Reader, the viewer recognizes the presence of a link -- the cursor changes when hovering over it -- but clicking it goes nowhere. Some other viewers such as pdf.js in Firefox appear to ignore it altogether.)

Comment 4

[Jonathan Kew [:jfkthame]]

@ajohnson Please let us know if you think you'll be able to address this in cairo in the immediate future, or should we try to find a workaround in Firefox for now?

Comment 5

[Jonathan Watt [:jwatt]]

Attachment: Example of PDF output on macOS when a link is missing

Comment 6

[Jonathan Kew [:jfkthame]]

Given that the PDF spec allows for "named destinations" as an alternative to the "explicit destination" that cairo is currently wanting to write here (but can't, because it doesn't have one), this hack:

diff --git a/gfx/cairo/cairo/src/cairo-pdf-interchange.c b/gfx/cairo/cairo/src/cairo-pdf-interchange.c
--- a/gfx/cairo/cairo/src/cairo-pdf-interchange.c
+++ b/gfx/cairo/cairo/src/cairo-pdf-interchange.c
@@ -1506,7 +1506,7 @@ cairo_pdf_interchange_write_forward_link
                                                                    TRUE,
                                                                    x, y);
            } else {
-               return _cairo_tag_error ("Link to dest=\"%s\" not found", link->dest);
+               _cairo_output_stream_printf(surface->object_stream.stream, "<>\n");
            }
        } else {
            cairo_pdf_interchange_write_explicit_dest (surface,

which makes us generate an empty string as a named destination seems to avoid the problem in initial testing. The generated PDF opens OK in browsers such as Firefox and Edge, and also in Adobe Reader; the resulting link just does nothing because when the reader attempts to look it up in the document catalog Dests entry, it won't be found.

[edit to add: Also checked that it works OK on Linux, at least in the Evince viewer on my system.]

@ajohnson, WDYT?

Comment 7

[Jonathan Kew [:jfkthame]]

Attachment: PDF output generated with the patch from comment 6

This is the result of "Save to PDF" with the site from comment 0; the "problem" link is "Internal link check in Shopify" on page 9.

Comment 8

[[e-mail]]

I tested the fix with Okular, Evince, and mupdf. Okular ignores the link. Evince and mupdf jump to the next page. The fix seems fine. I'll look into changing it in cairo so it still prints a warning when CAIRO_DEBUG_TAG is set but does not result in an error status on the surface.

Is it possible to notify the user if generating the PDF fails? There are other reasons it could fail such as unable to write to the file.

Comment 9

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

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

:dholbert, since you are the author of the regressor, bug 1729276, could you take a look?

For more information, please visit BugBot documentation.

Comment 10

[Jonathan Kew [:jfkthame]]

(In reply to [e-mail] from comment #8)

I tested the fix with Okular, Evince, and mupdf. Okular ignores the link. Evince and mupdf jump to the next page. The fix seems fine. I'll look into changing it in cairo so it still prints a warning when CAIRO_DEBUG_TAG is set but does not result in an error status on the surface.

Thanks for testing!

Is it possible to notify the user if generating the PDF fails? There are other reasons it could fail such as unable to write to the file.

Agreed, the user should be told that they haven't got the expected output. I've filed bug 1896070 for this.

Comment 11

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1895872 - Avoid generating a broken PDF when a link destination is not present in the generated output. r=#gfx-reviewers

Fix confirmed by ajohnson (see comment 8) to work around the reported issue.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f0275ae79eb1
Avoid generating a broken PDF when a link destination is not present in the generated output. r=gfx-reviewers,jrmuizel

Comment 14

[Sandor Molnar[:smolnar]]

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

Comment 15

[Ardelean Oana, Desktop QA [:oardelean]]

Reproducible on a 2024-05-09 Nightly build on Windows 10.
Verified as fixed on Firefox Nightly 128.0a1 and Firefox 127.0b2 on Windows 10, macOS 12, Ubuntu 22.