Fix browser_CaptivePortalWatcher.js to only wait once for the initial window to be activated at the end of test_detectedWithFocus

RESOLVED FIXED in Firefox 64

Status

()

defect
P1
normal
RESOLVED FIXED
8 months ago
8 months ago

People

(Reporter: Gijs, Assigned: Gijs)

Tracking

Trunk
Firefox 64
Points:
---

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

8 months ago
<@Gijs> dao: nhnt11: uh, so, my timeout happens at the end of the test_detectedWithFocus, and I'm a bit confused...
<@Gijs> I found a fix
<@Gijs> it basically does the same as https://searchfox.org/mozilla-central/source/browser/base/content/test/captivePortal/browser_CaptivePortalWatcher.js#60-63
<@Gijs> instead of https://searchfox.org/mozilla-central/source/browser/base/content/test/captivePortal/browser_CaptivePortalWatcher.js#116-117
<@Gijs> the current code times out, presumably because the default window gets focused/activated after win2 closes, and then the second promise waits forever for the initial window to get an activate event, which will never happen
<@Gijs> does that sound plausible? Was there a particular reason not to have those 2 codepaths match?
<nhnt11> uhh
<nhnt11> Gijs: that doesn't make complete sense to me, why does the default window get focus after win2 is closed?
<nhnt11> I'm pretty sure that test (in its current form) counts on the determinism of win1 getting focused after win2 is closed
<@Gijs> nhnt11: I don't know, but that comment in the first instance doesn't explain it either :P
<nhnt11> was that a mistake?
<nhnt11> ok
<nhnt11> Gijs: just re-looked
<nhnt11> I think you're right
Comment on attachment 9008380 [details]
Bug 1490642 browser_CaptivePortalWatcher.js should check for the default window to be activated instead of potentially waiting for it more than once, r?nhnt11

Nihanth Subramanya [:nhnt11] has approved the revision.
Attachment #9008380 - Flags: review+

Comment 3

8 months ago
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/c1200e5aa513
browser_CaptivePortalWatcher.js should check for the default window to be activated instead of potentially waiting for it more than once, r=nhnt11

Comment 4

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c1200e5aa513
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
You need to log in before you can comment on or make changes to this bug.