1663826 - gmail printing of image attachments doesn't work (closes too soon) because of use of window.open() + meta refresh + window.print()

Status: RESOLVED FIXED

(Toolkit :: Printing, defect, P1)

Description

[Neha Kochar [:neha]]

Attachment: printing jpg.mov

I'm on a Mac, using today's Nightly and trying to print a jpg from an email attachment does not work.
Attaching a screen recording to show with a test jpg. This is without Fission.
Browser console shows:
NS_ERROR_NOT_AVAILABLE:
which is very puzzling as it looks like an incomplete error message.

Comment 1

[Neha Kochar [:neha]]

Using a different jpg, I was able to get a better error from browser console:

[Exception... "Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIContentSniffer.getMIMETypeFromContent]" nsresult: "0x80040111 (NS_ERROR_NOT_AVAILABLE)" location: "JS frame :: resource:///modules/FaviconLoader.jsm :: onStopRequest :: line 298" data: no] FaviconLoader.jsm:298:24

Comment 2

[Sean Voisen (:svoisen)]

Will need to dig into what gmail is doing specifically here. Given the error in onStopRequest in FaviconLoader, maybe someone from frontend can investigate?

Comment 3

[Blake Winton (:bwinton) (:☕️)]

Yeah, that's a weird one. Maybe Gijs has an idea, or can redirect to someone who does?

Comment 4

[Sean Voisen (:svoisen)]

Gijs: Any ideas on this?

Comment 5

[:Gijs (he/him)]

Google loads the jpg in a new tab, but then navigates the context to the root of the email account (ie https://mail.google.com/mail/0/#inbox or similar). This is racy: if the notification for this navigation happening makes it to the parent process after we've managed to open the dialog, we close the dialog because the page navigated - if you're on page X and you navigate the tab, we close open dialogs for the previous webpage. If the notification arrives in the parent before we've managed to open the dialog, we don't close it, and somehow we still get the right printing document? I don't fully understand how that works - because you can see that behind the dialog, the tab still ends up loading the main mail account.

Then after loading this other page, google calls window.close() on the window it opened using window.open() and closes the tab. If the print was still up and we were still spinning the event loop (after the fixes from bug 1662975 and friends), that would block the window.close() call, I think.

Though I'm curious: would this mean that in the window.open case with the tabdialogbox open for one tab that is trying to print, the opening tab would be "hung", because we're blocking the event loop over there? Should deselecting the tab cancel the print dialog?

The real issue here is that google is redirecting the document at all (no idea why they do that...), but we probably need to cope with it somehow anyway. We certainly do so if you turn off the modal print stuff. I expect this means there is some difference between how we dealt with the window-modal dialog and how we do the event loop spinning in bug 1662975, but I'm unsure what that is. Emilio?

The faviconloader error is completely unrelated.

Comment 6

[:Gijs (he/him)]

Bah, having re-read my own comment, I guess maybe the diff with the "old" print code is just that we didn't use to abort printing on navigation, and now we do?

(In reply to :Gijs (he/him) from comment #5)

if you're on page X and you navigate the tab, we close open dialogs for the previous webpage.

The tabdialog code as of 82 supports a flag to not do this for same-origin navigations, which we can use to wallpaper this. I just tested this, and it works. However, the same bug would still occur if the navigation was non-same-origin.

Though I'm curious: would this mean that in the window.open case with the tabdialogbox open for one tab that is trying to print, the opening tab would be "hung", because we're blocking the event loop over there? Should deselecting the tab cancel the print dialog?

With the wallpaper above, the answer here is: no, the previous tab is not hung.

So I guess the question is... do we need a new flag on the tabdialogbox to just completely ignore navigations? Feels pretty yucky, also because I'd definitely expect user-triggered navigations to clear the print dialog box - it'd be weird if you could navigate the window "behind" the print dialog box! We could potentially detect this based on the triggering principal of the navigation being system principal, but that feels (a) very yucky and (b) we may not have access to it when the tabdialogbox gets notified.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

window.print() is supposed to block the page from doing anything else until you close the dialog... So that's probably not working quite right. Looking now.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: Reduced test-case?

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Attachment: test-case that works

Ah, so it is the front-end which is actually aborting the dialog.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Attachment: Actual reduced test-case.

This is reconstructed from stepping through it with rr.

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Attachment: More extended test-case.

This also includes how they print in Chrome. It's not clear why they are doing what they're doing...

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Mike, do we have any mailing list with the gmail folks? Is there any chance we can ask them about why are they printing like this (window.open() + a meta refresh + window.print()) rather than how they print in chrome (calling window.print() on an iframe?).

We can possibly hack around this somehow, I guess, but not clear what's the best way to do that...

Comment 13

[Emilio Cobos Álvarez (:emilio)]

I expect this means there is some difference between how we dealt with the window-modal dialog and how we do the event loop spinning in bug 1662975, but I'm unsure what that is.

To reply to this, the issue is just that we close the dialog when we navigate indeed, as you noticed earlier. And window.print() is working as expected, since the navigation gets triggered before even (via <meta> refresh).

Comment 14

[:Gijs (he/him)]

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

We can possibly hack around this somehow, I guess, but not clear what's the best way to do that...

So I can see a few options:

0. the wallpaper from comment #6 of using the existing "keep the print dialog open for same-origin navigations". This has downsides in that even user navigations on the same domain would be possible while the dialog stays up.
1. add another flag to tabdialogbox to opt the print dialog out of being closed by non-system-principal and/or non-userinteraction navigations
2. block the meta refresh and/or navigation altogether while we're dealing with printing (unsure if this is web compatible) except for system principal ones
3. detect if we're printing a window with .opener and we're navigating (with a non-system principal), and do something different based on that?

Comment 15

[Mike Taylor [:miketaylr]]

I sent an email to our partner list.

Comment 16

[Sean Voisen (:svoisen)]

Moving this the frontend component for tracking given it sounds like the fix will be there in the dialog. Would like to get input from gmail, but we may not want to wait for them to find a solution on their side. Should we proceed forward from one of the options proposed in comment 14?

Comment 17

[:Gijs (he/him)]

(In reply to Sean Voisen (:svoisen) from comment #16)

Moving this the frontend component for tracking given it sounds like the fix will be there in the dialog. Would like to get input from gmail, but we may not want to wait for them to find a solution on their side. Should we proceed forward from one of the options proposed in comment 14?

So I think options (2) and (3) are out because the navigation gets initiated before the .print() call so we cannot block it, based on comment #13.

But I'm pretty worried about both of the other options. Navigation should close this dialog. And in fact, this same thing happens in Chrome - you can try it yourself if you open pagea.com, run a setTimeout(() => { location.href = "https://pageb.com/"; }, 5000), and hit accel-p before the timeout is up. Same thing for same-origin navigations.

So if Google ran their "print for other browsers" code against Chrome, and navigation actually happened, then they would have the same problem, AIUI. Emilio's testcase breaks in Chrome - the document.write does not successfully create a document.body, for whatever reason. If I just append the div to doc instead, the testcase still does not navigate. It seems the meta refresh is overwritten by the other code and Chrome doesn't navigate.

Emilio, any chance there's a difference in behaviour in how the meta refresh is processed as part of document.write or something, and that aligning with Chrome there in a way that incidentally fixes this bug is possible?

Comment 18

[:Gijs (he/him)]

Otherwise, it'd be interesting if we could ship some kind of web-compat intervention that got us the Chrome printing behaviour for gmail...

Comment 19

[Emilio Cobos Álvarez (:emilio)]

[Tracking Requested - why for this release]:

So I figured why the meta refresh isn't navigating in release, and it is a regression, we're not supposed to navigate while printing. This is the main regression.

As noted elsewhere though, this is allowed by other browsers, in some cases, at least.

Comment 20

[Emilio Cobos Álvarez (:emilio)]

The fact that we don't block the navigation is at least caused by bug 1636728.

We're stuck in a nested event loop so usually the page isn't allowed to navigate anyhow, but since <meta refresh> happens off a timer it catches us off-guard.

Comment 21

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1663826 - Keep freezing navigation during window.print() for compat. r=smaug

This broke in bug 1636728 because we started setting the bit in the
cloned docshell rather than the original one.

Behavior in other browsers seems to be a bit all over the place, but for
now keeping our behavior during window.print() seems sane.

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/168ce28f0049
Keep freezing navigation during window.print() for compat. r=smaug

Comment 23

[Ryan VanderMeulen [:RyanVM]]

This seems like a pretty bad regression to ship with. It seems really unfortunate that this only got on my radar now when we're just about to build RC2 for 81.0. I'm on the fence about this being a release blocker - thoughts?

Comment 24

[Emilio Cobos Álvarez (:emilio)]

So, this is less visible for the old print UI, because it navigates but you can still print... so it's probably fine-ish to ship with that? Yet other sites could be affected... not sure, I'm also a bit on the fence :)

Comment 25

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/168ce28f0049

Comment 26

[Blake Winton (:bwinton) (:☕️)]

Can we replicate this on the old UI? The bug is marked [old-ui-], so I thought it was only for the new UI (in which case it's not really a regression, and we don't need to worry about it)…

Comment 27

[Emilio Cobos Álvarez (:emilio)]

Yes. With the old UI you can print, but instead of the page with the image you see a gmail tab load behind you.