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

RESOLVED FIXED in Firefox -esr52

Status

()

defect
RESOLVED FIXED
3 years ago
2 years ago

People

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

Tracking

({intermittent-failure})

unspecified
mozilla55
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr52 fixed, firefox53 wontfix, firefox54 fixed, firefox55 fixed)

Details

(Whiteboard: [stockwell fixed])

Attachments

(2 attachments)

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: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Mike, can you please request Beta/ESR52 approval on this when you get a chance?
Assignee: nobody → mconley
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.