1656022 - Print Edit WE no longer saves all images when saving to PDF, because requests introduced by bug 1648064 are blocked (was: tabs.saveAsPDF() API no longer saves all images)

Status: RESOLVED INVALID

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

Description

[dw-dev]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Firefox/78.0

Steps to reproduce:

I used my Print Edit WE extension on Firefox Nightly 81.0a1 (2020-07-29) to save a Wikipedia web page as a PDF. The web page was saved as a PDF, but most of the images were missing.

This works correctly on Firefox 56 - 79, but not on Firefox 80.0b1 or Firefox Nightly 81.0a1. I implemented the tabs.saveAsPDF() API and that code has not changed recentlt. So something must have changed in the backend print-to-pdf code (printSettings.printToFile = true; printSettings.outputFormat = Ci.nsIPrintSettings.kOutputFormatPDF;).

Actual results:

The Wikipedia web page was saved as a PDF. The text was saved correctly, but only one of the images was saved. All of the other images were NOT saved.

Expected results:

All of the images should have been saved.

Comment 1

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

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

I can repro this, but it doesn't happen with the "native" print / print preview, can you confirm that you see the same?

Bug 1648064 changed all our setup for printed images and it matches the regression range (should be in 80), so it has 99% chances to be the regressing change...

Comment 3

[Simona Badau, Desktop QA]

Investigated the issue, I could reproduce it when printing with the Print Edit WE extension by choosing to save to pdf this Wikipedia page:
https://en.wikipedia.org/wiki/COVID-19_pandemic

I managed to find a regression range, here are the results:

• first known bad: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=1600e73bdd901c5ac4b831c43e29f73d561aa816&tochange=18cfd7807be8abbb7da9ce1a75eba6092163164e

• last known good: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=71bb0ab476bb2093447780e9fc6d7f55abeadcd4&tochange=18cfd7807be8abbb7da9ce1a75eba6092163164e

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Yeah, so bug 1648064, as expected.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

[Tracking Requested - why for this release]: We shouldn't ship this regression. Depending on the complexity of the fix we may want to back out the regressing bug from 80.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Ok, so I tracked this down using rr, and here's what's going on:

• The relevant images have a srcset for a hidpi and a lowdpi image.
• The print document has a lower DPI, so in some cases that triggers an image request as of bug 1648064.
• The image request is erroring, which leaves us with the broken image.

Now, why is the image request erroring? I had to jump to the parent process to figure out. The request was being canceled here.

Digging a bit more (I'm not all that familiar with the extensions code, so I had to figure out how to get the extension source and such... thanks for making it GPL dw-dev!), I see that the extension has code like this:

    chrome.webRequest.onBeforeRequest.addListener(
    function(details)
    {
        if (debugEnable) webRequestLog("oBR-1",details);
        
        if (typeof tabCurModes[details.tabId] == "undefined" || tabCurModes[details.tabId] < 0) return;  /* not blocking HTTP */
        
        /* Before HTTP request */
        
        if (debugEnable) webRequestLog("oBR-2",details);
        
        if (details.type == "main_frame")  /* main-frame request (reload or navigation) */
        {
            tabCurModes[details.tabId] = -2;  /* url committed */
            
            updateBrowserAction(details.tabId,details.url);
            
            updateContextMenus();
            
            return;  /* leave page */
        }
        else  /* sub-frame or resource request */
        {
            return { cancel: true };  /* cancel request */
        }
    }
    ,{ urls: ["<all_urls>"] },["blocking"]);

That return { cancel: true }; is what is making the image load error, afaict. I'm not all that familiar with the webrequest APIs so other of the cancel stuff goes on.

So given that, I think this is a bug in the extension. There's no reason why those image loads should be getting blocked. It's perfectly legit to do something like:

<picture>
  <source srcset="print.png" media="print">
  <source srcset="screen.png" media="not print">
  <img>
</picture>

Or such with other width / height settings, for example, to serve more print-friendly images.

Bug 1655933 will help here a bit. The srcset case is... interesting (if we have a higher-res image maybe we should just reuse it as we were doing before my patch, though it seems hard to prove that it's the same image without triggering the load :)).

Reporter, do you agree with that diagnostic? We could revert my changes for Firefox 80 if necessary if it helps you, though probably not for 81.

Comment 7

[dw-dev]

I have just spent a couple of hours doing some more testing to understand the differences in behaviour of:

• File > Print Preview
• File > Print
• Save PDF extension - Click toolbar button
• Print Edit WE extension - Click toolbar button > Save > Save As PDF

All tests were done with a Wikipedia page: https://en.wikipedia.org/wiki/Main_Page

Here are the results:

• File > Print Preview / File > Print - Normally, the page is correctly displayed/printed with Firefox 78 and Nightly 81.0a1 (2020-07-30).

• Save PDF extension - Normally, the page is correctly saved in PDF with Firefox 78 and Nightly 81.0a1 (2020-07-30).

• Print Edit WE extension - Normally, the page is correctly saved in PDF with Firefox 78, but most images (except for 1 or 2) are not saved with Nightly 81.0a1 (2020-07-30).

• In all cases - If you Work Offline and Clear Recent History after loading the page, then with Firefox 78 the images are displayed/printed/saved, but with Nightly 81.0a1 (2020-07-30) the images are not displayed/printed/saved.

In summary:

• When previewing, printing and saving as PDF, Firefox 78 does not re-fetch the images from the network or cache, but Nightly 81.0a1 (2020-07-30) does re-fetch the images from the network or cache.

• When editing a page, Print Edit WE blocks all network requests using the webRequest APIs, to avoid the possibility of editing actions accidentally triggering network transactions:

  • With Firefox 78 this is not a problem - previewing, printing and saving as PDF all work.
  • With Nightly 81.0a1 (2020-07-30) this is a big problem - previewing, printing and saving as PDF are all missing images.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

That is expected. After bug 1648064, the print action behaves more like a regular page load from the point of view of image loading. This is needed both for <img loading=lazy>, and <source media>, among others.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

To clarify: that is exactly what that bug changed, pretty intentionally. That's also how printing web fonts, background-images, list-style-image, etc already worked.

We could try to keep the old image and trigger a new load, maybe except if it's the same source, or something, but that seems like more printing specialness which I'd rather avoid, and doesn't fix the srcset case explained above.

Comment 10

[dw-dev]

I haven't noticed any problems with fonts, background-images or list-style-images when saving pages as PDF using Firefox 78 and earlier versions - and I haven't had any problem reports about this from users - which seems a bit odd.

Is it only images and fonts that are re-fetched? Why aren't they fetched from cache?

With regards to Print Edit WE, it will be difficult to allow image and font requests, but still block all other requests.

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Is it only images and fonts that are re-fetched? Why aren't they fetched from cache?

Yes, that's right. For example here's a test-case that doesn't print a background image with Print Edit WE, but prints with the native dialog (if printing background images): https://crisal.io/tmp/print-bg-image.html

I see the add-on does some munging with CSS media queries too, I think that's what may cause most of the stuff to be in the cache already while editing...

As of right now, yes, it's only images or fonts... Other resources like CSS need to be fetched during the normal page load, so we reuse them directly. As of why they're not fetched from the cache, in this case it is because of a combination of two things:

• The print document having a lower dpi, and thus matching different srcset values, or...
• The images being not cacheable, and loaded on the print document, and thus not being able to reuse the cache from the main document. I fixed that in bug 1655933, which should be in the next nightly.

With regards to Print Edit WE, it will be difficult to allow image and font requests, but still block all other requests.

I'm sorry to hear that, is there something that I could help with? Not particularly familiar with webRequest APIs but...

Comment 12

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

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

Comment 13

[dw-dev]

I have redesigned Print Edit WE so that the 'Save As PDF' command now works with Firefox 80.0b1 and Firefox Nightly 81.0a1 (2020-07-31).

Print Edit WE 26.4 has been submitted to Mozilla for review and release.

So from my point of view, the bug I originally reported has been fixed - and was in Print Edit WE rather than in Firefox.

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Great, thanks! Fyi (as I saw an special case for this while going through Print Edit WE's source), bug 1653354 should make "save to pdf" available on Firefox on Mac.

Thanks again for filing this!