Closed
Bug 1486824
Opened 5 years ago
Closed 5 years ago
Make BrowserTestUtils.openNewBrowserWindow() delegate to window.OpenBrowserWindow
Categories
(Firefox :: General, enhancement, P1)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox 63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: Gijs, Assigned: Gijs)
References
Details
Attachments
(1 file)
This will: - reduce code duplication (the current implementation does much the same thing as OpenBrowserWindow) - increase test coverage of new window opening + loading of about:newtab/about:home - facilitate starting to make decisions about the initial URL passed to new browser windows without having to have special cases for tests (see bug 1362774 comment 150 and further) - reduce the number of paths we have that manually open browser.xul and that we therefore have to update/audit if we try to change some of the window arguments handling (e.g. bug 1485961) The main obvious issues were with callsites passing positioning information. I rewrote those successfully already. With those, I have an initial trypush at https://treeherder.mozilla.org/#/jobs?repo=try&revision=ee9de926da36f8c3017386f14e18ac8e404c5f2c&selectedJob=195811711 . There are less than 5 remaining tests with issues, and I hope to have a patch later this week. Nihanth is helping with the captive portal issue.
Assignee | ||
Comment 1•5 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=fd78d10442e9d114ae93c9c356248dd7b73f56b3
Assignee | ||
Comment 2•5 years ago
|
||
Updated•5 years ago
|
Priority: -- → P1
Comment 3•5 years ago
|
||
Comment on attachment 9004614 [details] Bug 1486824 - change BrowserTestUtils to just call OpenBrowserWindow so it does the same thing as opening a window normally, r?nhnt11,mconley Nihanth Subramanya [:nhnt11] has approved the revision.
Attachment #9004614 -
Flags: review+
Assignee | ||
Comment 4•5 years ago
|
||
Looks like browser/base/content/test/plugins/browser_private_clicktoplay.js needs fixing, too.
Assignee | ||
Comment 5•5 years ago
|
||
(In reply to :Gijs (he/him) from comment #4) > Looks like browser/base/content/test/plugins/browser_private_clicktoplay.js > needs fixing, too. https://treeherder.mozilla.org/#/jobs?repo=try&revision=d694a6a8ab3c6fe71c4a19cdb4c4170786b1f881 looks green, with the remaining orange apparently caused by bug 1483325 and some other intermittents.
Comment 6•5 years ago
|
||
Comment on attachment 9004614 [details] Bug 1486824 - change BrowserTestUtils to just call OpenBrowserWindow so it does the same thing as opening a window normally, r?nhnt11,mconley Mike Conley (:mconley) (:⚙️) has approved the revision.
Attachment #9004614 -
Flags: review+
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/4e195f7b2d6b change BrowserTestUtils to just call OpenBrowserWindow so it does the same thing as opening a window normally, r=mconley,nhnt11
Comment 8•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4e195f7b2d6b
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Comment 9•5 years ago
|
||
Backed out for clipboard failures on OSX browser_editcontrols_update. Backout link: https://hg.mozilla.org/mozilla-central/rev/1bd0f0baab41d587e900da8797467801d71568a5 Push link: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=4e195f7b2d6b8c12aba04ef69401e9af65daa05a&filter-resultStatus=testfailed Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=196632547&repo=mozilla-central&lineNumber=1604 This push might also affect some bc failures like "browser_privatebrowsing_about.js | Uncaught exception - TypeError: content.document.getElementById(...) is null, can't access property "checked" of it " Log link : https://treeherder.mozilla.org/logviewer.html#?job_id=196638260&repo=autoland&lineNumber=3769
Flags: needinfo?(gijskruitbosch+bugs)
Updated•5 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Firefox 63 → ---
Assignee | ||
Updated•5 years ago
|
Flags: needinfo?(gijskruitbosch+bugs)
Comment 10•5 years ago
|
||
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/96b5a88ac004 change BrowserTestUtils to just call OpenBrowserWindow so it does the same thing as opening a window normally, r=mconley,nhnt11
Comment 11•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/96b5a88ac004
Status: REOPENED → RESOLVED
Closed: 5 years ago → 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Comment 12•5 years ago
|
||
Backed out changeset 96b5a88ac004 (Bug 1486824) for causing frequent failures e.g: browser/components/privatebrowsing/test/browser/browser_privatebrowsing_about.js a=backout Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=96b5a88ac0041a1bd382a731749e1fca9aa911ed&filter-searchStr=Linux%20x64%20debug%20Mochitests%20with%20e10s%20test-linux64%2Fdebug-mochitest-browser-chrome-e10s-11%20M-e10s(bc11)&selectedJob=196787659 Failure logs: https://treeherder.mozilla.org/logviewer.html#?job_id=196787659&repo=autoland&lineNumber=4027 https://treeherder.mozilla.org/logviewer.html#?job_id=196840060&repo=autoland&lineNumber=3676 Backout: https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=63d2134f95878cabfb89c7849a1baad2ef139fd0&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=runnable
Status: RESOLVED → REOPENED
status-firefox63:
fixed → ---
Flags: needinfo?(gijskruitbosch+bugs)
Resolution: FIXED → ---
Target Milestone: Firefox 63 → ---
Comment 13•5 years ago
|
||
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c4b770468984 change BrowserTestUtils to just call OpenBrowserWindow so it does the same thing as opening a window normally, r=mconley,nhnt11
Comment 14•5 years ago
|
||
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c995c3a15034 review comment follow-up: change params for openNewBrowserWindow to explicitly check for URL, r=nhnt11
Assignee | ||
Comment 15•5 years ago
|
||
(In reply to Pulsebot from comment #14) > Pushed by gijskruitbosch@gmail.com: > https://hg.mozilla.org/integration/mozilla-inbound/rev/c995c3a15034 > review comment follow-up: change params for openNewBrowserWindow to > explicitly check for URL, r=nhnt11 Apologies for the separate commits - but in essence, I changed the tests some more here to explicitly wait for about:privatebrowsing as part of waiting for the initial window to open. In my trypush, even after repeated retriggers, I could no longer reproduce the orange that got me backed out last week, so let's hope it sticks this time...
Flags: needinfo?(gijskruitbosch+bugs)
Comment 16•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c4b770468984 https://hg.mozilla.org/mozilla-central/rev/c995c3a15034
Status: REOPENED → RESOLVED
Closed: 5 years ago → 5 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
You need to log in
before you can comment on or make changes to this bug.
Description
•