Closed Bug 1308820 Opened 8 years ago Closed 8 years ago

Intermittent browser/base/content/test/tabs/browser_tabSwitchPrintPreview.js | Found an unexpected at the end of test run -

Categories

(Core :: Print Preview, defect)

Product:
Core
Component:
Print Preview
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox-esr52 --- fixed
firefox53 --- wontfix
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: mconley)

References

Details

(Keywords: intermittent-failure, Whiteboard: [stockwell fixed])

Attachments

(2 files)

mozilla-test-fail-screenshot_aUbp81.png
247.54 KB, image/png
Details
Bug 1308820 - Make sure to close the print progress dialog if print preview is exited.
59 bytes, text/x-review-board-request
Gijs
: review+
gchang
: approval-mozilla-beta+
gchang
: approval-mozilla-esr52+
Details
Here's a screenshot of what happens when the test fails. Looks like there's a print progress dialog still being displayed. We should probably wait for that to close before finishing the test.
Whiteboard: [stockwell needswork]
Comment on attachment 8863427 [details] Bug 1308820 - Make sure to close the print progress dialog if print preview is exited. https://reviewboard.mozilla.org/r/135198/#review138444 r=me, thanks for fixing this! (While reviewing, I noticed that the test had been adjusted for e10s-multi, and Ni'd Gabor in bug 1315042 about this. Perhaps this fix will be enough to remove the workaround added there - but either way, we should get rid of that workaround...) ::: toolkit/components/printing/content/printUtils.js:660 (Diff revision 1) > + // If we were still showing a print progress dialog, tell it to close. > + if (this._webProgressPP && this._webProgressPP.value) { > + this._webProgressPP.value.onStateChange(null, null, > + Components.interfaces.nsIWebProgressListener.STATE_STOP, 0); > + } Should we factor this out into a helper for use here and in the earlier identical code for the re-entrant case?
Attachment #8863427 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8863427 [details] Bug 1308820 - Make sure to close the print progress dialog if print preview is exited. https://reviewboard.mozilla.org/r/135198/#review138444 > Should we factor this out into a helper for use here and in the earlier identical code for the re-entrant case? Yeah, sounds good. I'll do that before landing. Thanks!
Pushed by mconley@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c0fbc43ebb83 Make sure to close the print progress dialog if print preview is exited. r=Gijs
https://hg.mozilla.org/mozilla-central/rev/c0fbc43ebb83
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Mike, can you please request Beta/ESR52 approval on this when you get a chance?
Assignee: nobody → mconley
status-firefox53: --- → wontfix
status-firefox54: --- → affected
status-firefox-esr52: --- → affected
Flags: needinfo?(mconley)
Comment on attachment 8863427 [details] Bug 1308820 - Make sure to close the print progress dialog if print preview is exited. Approval Request Comment [Feature/Bug causing the regression]: Tests. [User impact if declined]: None, I don't think. Maybe there's a corner case where print preview / print progress dialogs will continue to exist after print preview is exited that this will fix, but I doubt it. Mostly, this is to fix an intermittent. [Is this code covered by automated tests?]: Yep. [Has the fix been verified in Nightly?]: Yep. [Needs manual test from QE? If yes, steps to reproduce]: Nope. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: We're making sure to clean up after ourselves after we exit print preview. [String changes made/needed]: None.
Flags: needinfo?(mconley)
Attachment #8863427 - Flags: approval-mozilla-beta?
See Also: → 1361992
Whiteboard: [stockwell needswork] → [stockwell fixed]
Comment on attachment 8863427 [details] Bug 1308820 - Make sure to close the print progress dialog if print preview is exited. Fix an intermittent failure related to print preview. Beta54+. Should be in 54 beta 6.
Attachment #8863427 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8863427 [details] Bug 1308820 - Make sure to close the print progress dialog if print preview is exited. This went out with 54 without causing any known regressions and OrangeFactor data confirms that the fix was effective. If possible, it would be nice to get it on ESR52 as well where this intermittent continues to happen.
Attachment #8863427 - Flags: approval-mozilla-esr52?
Comment on attachment 8863427 [details] Bug 1308820 - Make sure to close the print progress dialog if print preview is exited. Fix an intermittent failure. Let's uplift it to ESR52.3.
Attachment #8863427 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: