Closed Bug 1385818 Opened 2 years ago Closed 2 years ago

Convert dom/tests/mochitest/chrome/test_popup_blocker_chrome.xul to comply with new data: URI inheritance model

Categories

(Core :: DOM: Security, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: ckerschb, Assigned: ckerschb)

References

Details

(Whiteboard: [domsecurity-backlog1])

Attachments

(1 file, 1 obsolete file)

This one seems more complicated because SimpleTest.waitForFocus() seems not to fire, probably we can use something like < ...onload="parent.doTest()"> instead.
Blocks: 1337269
Priority: -- → P2
See Also: → 1385273
Whiteboard: [domsecurity-backlog1]
Attached patch popup_blocker_chrome.patch (obsolete) — Splinter Review
Here is the WIP patch I came up with, but that still needs work (see comment 0).
Assignee: nobody → ckerschb
Status: NEW → ASSIGNED
Smaug, I know you are not accepting r? requests at the moment, but you are already familiar with what we are doing here. Any chance you could review that test update? 

Anyway, for some reason SimpleTest.waitForFocus() does not fire when using an external file instead of the data: URI. I tried to debug SimpleTest.waitForFocus(), but it seems that SimpleTest not even receives the load() notificiation and waitForFocus only advances if 'loaded' and 'focus' are true [1]. Eventually I gave up and just rewrote the test to rely on <body onfocus ...> instead.

[1] https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/tests/SimpleTest/SimpleTest.js#789
Attachment #8891941 - Attachment is obsolete: true
Flags: needinfo?(bugs)
Flags: needinfo?(bugs)
Attachment #8892451 - Flags: review+
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/385dd315c9ca
Convert dom/tests/mochitest/chrome/test_popup_blocker_chrome.xul to comply with new data: URI inheritance model. r=smaug
Keywords: checkin-needed
Backed this out for failures like https://treeherder.mozilla.org/logviewer.html#?job_id=120084938&repo=mozilla-inbound which seem to affect all Windows mochitest-chrome runs and all OSX opt mochitest-chrome runs.


https://hg.mozilla.org/integration/mozilla-inbound/rev/40a251b511b32f5938a42f92caf4947494025b7b
Flags: needinfo?(ckerschb)
Backout by kwierso@gmail.com:
https://hg.mozilla.org/mozilla-central/rev/fec8d7259005
Backed out changeset 385dd315c9ca for failures in test_popup_blocker_chrome.xul a=backout
(In reply to Wes Kocher (:KWierso) from comment #4)
> seem to affect all Windows mochitest-chrome runs and all OSX
> opt mochitest-chrome runs.

Worked locally on linux - rrrh. I'll have a look.
Flags: needinfo?(ckerschb)
Probably onfocus is not firing correctly. Anyway, I tried locally on mac and it seems we can just use onload instead. Added some debugging information, let's see how that goes:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2200e56e5eb6c30562fe31310319cda460ea0f38
Pushed by mozilla@christophkerschbaumer.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2b1608415e15
Convert dom/tests/mochitest/chrome/test_popup_blocker_chrome.xul to comply with new data: URI inheritance model. r=smaug
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #7)
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=2200e56e5eb6c30562fe31310319cda460ea0f38

It seems I forgot to hit 'save' before. Anyway:
It seems we can simply rely on onload vs onfocus. Let's do that instead than.
https://hg.mozilla.org/mozilla-central/rev/2b1608415e15
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.