Closed Bug 1404176 Opened 2 years ago Closed 2 years ago

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

Categories

(Toolkit :: Printing, defect, P1)

55 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla59
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- wontfix
firefox56 --- wontfix
firefox57 --- wontfix
firefox58 --- verified
firefox59 --- verified

People

(Reporter: karlt, Assigned: mlongaray)

References

Details

(Keywords: regression)

Attachments

(1 file)

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.
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?
Flags: needinfo?(mlongaray)
Priority: -- → P1
Actually, at this point, 57 is pretty much wrapped, so moving it from fix-optional to wontfix.
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?
Flags: needinfo?(mlongaray) → needinfo?(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?
Flags: needinfo?(mconley) → needinfo?(mlongaray)
Sounds like a plan! I'll probably be writing it on Monday or as soon as I get the cycle.
Flags: needinfo?(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 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!
Attachment #8927868 - Flags: review?(mconley) → review+
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/676474b0b6b8
Remove aborted print preview browser before bailing out. r=mconley
https://hg.mozilla.org/mozilla-central/rev/676474b0b6b8
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Please request Beta approval on this when you get a chance.
Assignee: nobody → mlongaray
Flags: needinfo?(mlongaray)
Version: Trunk → 55 Branch
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
Flags: needinfo?(mlongaray)
Attachment #8927868 - Flags: approval-mozilla-beta?
Comment on attachment 8927868 [details]
Bug 1404176 - Remove aborted print preview browser before bailing out.

Fix a print preview related issue. Beta58+.
Attachment #8927868 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
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
QA Whiteboard: [bugday-20171129]
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.