1404176 - unable to close print preview after attempt to print about:config

Status: VERIFIED FIXED

(Toolkit :: Printing, defect, P1)

Description

[Karl Tomlinson (:karlt)]

unable to close print preview after attempt to print about:config

1. Open about:config in a new tab
2. Hamburger -> Print
3. Close "There was an unexpected problem while printing.",
4. Cancel "Print Preview", if necessary.
5. Switch back to Nightly Start Page.
6. Hamburger -> Print
7. Try to close print preview with "Close" button.

Expected:
Print preview closes.

Actual:
No response, apart from change in highlight on "Close" button.

20170501153126 8528bf8f2ecfa90a451ba3e74c7426a3f3596212 bad
20170501150922 9bcc235d3a5d545f599fa5bed1af8208076331f9 good

Regression from 8528bf8f2ecfa90a451ba3e74c7426a3f3596212 for bug 1332386.

Comment 1

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

Yeah, this isn't amazing. We should fix this as soon as we can - though I don't think it's a 57 blocker (we've been shipping this bug since 55).

Hey mlongaray, got the cycles to look at this?

Comment 2

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

Actually, at this point, 57 is pretty much wrapped, so moving it from fix-optional to wontfix.

Comment 3

[Matheus Longaray (:mlongaray)]

Hey Mike, I did a quantum (fast) look at this today and two errors came up in Browser Console, see below. I'll probably have time to dive deeper here in 1 week or so (+).


Error #1:

1. Open about:config in a new tab
2. Hamburger -> Print
3. Close "There was an unexpected problem while printing."
4. Cancel "Print Preview", if necessary.

This error happens when the content process initializes print preview (http://searchfox.org/mozilla-central/source/toolkit/content/browser-content.js#720). So when printUtils monitors the loading progress of the document and receives a STATE_STOP message (transition flag), it tries to get a reference of print preview toolbar to disable all toolbar elements when updating. See here: http://searchfox.org/mozilla-central/source/toolkit/components/printing/content/printUtils.js#450. The problem is that printPreviewTB is null and so produces a type error. Console output error below.

[Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIWebBrowserPrint.printPreview]"  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: chrome://global/content/browser-content.js :: printPreviewInitialize :: line 720"  data: no]
TypeError: printPreviewTB is null  printUtils.js:451:11


Error #2:
5. Switch back to Nightly Start Page.
6. Hamburger -> Print
7. Try to close print preview with "Close" button.

For this second scenario, when we exit print preview in printUtils, browser message manager is apparently null. See here: http://searchfox.org/mozilla-central/source/toolkit/components/printing/content/printUtils.js#701. Console output error below.

TypeError: browserMM is undefined printUtils.js:701:7
exitPrintPreview chrome://global/content/printUtils.js:701:7


Mike, do you see any reason why (and if) these errors are somewhat tied together producing a bad state in printUtils.js?

Comment 4

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

(In reply to Matheus Longaray (:mlongaray) from comment #3)
> Mike, do you see any reason why (and if) these errors are somewhat tied
> together producing a bad state in printUtils.js?

Yeah, these seem super related. It sounds like enterPrintPreview is hitting an error and not aborting / cleaning itself up fully. Probably enough to keep what was going to be the print preview tab for the about:config tab inside the _ppBrowsers Set().

Then, when we try to exitPrintPreview, we loop over _ppBrowsers, and hit the one from the aborted about:config print preview, which never got fully created, and therefore doesn't have a message manager.

So I suspect what we want to do is remove the ppBrowser from the aborted print preview before bailing out here:

http://searchfox.org/mozilla-central/rev/aa1343961fca52e8c7dab16788530da31aab7c8d/toolkit/components/printing/content/printUtils.js#614-615

Seems like a pretty straight-forward patch (famous last words!) - mlongaray, feel like writing it, and I'll review?

Comment 5

[Matheus Longaray (:mlongaray)]

Sounds like a plan! I'll probably be writing it on Monday or as soon as I get the cycle.

Comment 6

[Matheus Longaray (:mlongaray)]

Attachment: Bug 1404176 - Remove aborted print preview browser before bailing out.

This patch clears out set of print preview browsers before bailing out when
there is an error while putting the document into print preview mode.

Review commit: https://reviewboard.mozilla.org/r/199146/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/199146/

Comment 7

[Matheus Longaray (:mlongaray)]

Sorry the delay, Mike. Let me know next steps. I gave a few rounds of manual testing and it did the job (looks like my Mercurial access was disabled due to account inactivity so I could not trigger a try run).

Comment 8

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

Comment on attachment 8927868 [details]
Bug 1404176 - Remove aborted print preview browser before bailing out.

https://reviewboard.mozilla.org/r/199146/#review204212

Looks great, thanks mlongaray!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/676474b0b6b8
Remove aborted print preview browser before bailing out. r=mconley

Comment 10

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/676474b0b6b8

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this when you get a chance.

Comment 12

[Matheus Longaray (:mlongaray)]

Comment on attachment 8927868 [details]
Bug 1404176 - Remove aborted print preview browser before bailing out.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1332386
[User impact if declined]: User may not be able to close print preview
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: No
[Needs manual test from QE? If yes, steps to reproduce]: Yes, see https://bugzilla.mozilla.org/show_bug.cgi?id=1404176#c0
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Low risk
[Why is the change risky/not risky?]: It clears set of print preview browsers before bailing out when there is an error while putting the document into print preview mode
[String changes made/needed]: None

Comment 13

[Gerry Chang [:gchang]]

Comment on attachment 8927868 [details]
Bug 1404176 - Remove aborted print preview browser before bailing out.

Fix a print preview related issue. Beta58+.

Comment 14

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/releases/mozilla-beta/rev/14022278b66d

Comment 15

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 58.0a1 (2017-09-28)on Windows 10 64 Bit!

This bug's fix is verified with Latest Beta and Latest Nightly!

Build ID  : 20171130160223
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0


Build ID  : 20171201100115
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:59.0) Gecko/20100101 Firefox/59.0

Comment 16

[Ciprian Georgiu, Desktop QA]

Thanks Mohammad for your testing efforts!

I also managed to reproduce this bug using the STR from comment 0, on an affected Nightly build 58.0a1 (2017-09-28).

Verified fixed on latest Nightly 59.0a1 (2017-12-05) and Beta 58.0b9 (20171204150510) under Windows 10 x64 and Ubuntu 16.04 x64.