Closed
Bug 1246843
Opened 8 years ago
Closed 8 years ago
Set a default page in mozscreenshots mochitest
Categories
(Testing :: mozscreenshots, defect)
Testing
mozscreenshots
Tracking
(firefox47 fixed)
RESOLVED
FIXED
| Tracking | Status | |
|---|---|---|
| firefox47 | --- | fixed |
People
(Reporter: bgrins, Assigned: bgrins)
References
Details
Attachments
(1 file)
The mochitest harness (in browser-test.js) is forcing the tab to be set to about:blank, but that causes some weirdness in the DevTools runner. about:home is a much more interesting page anyway.
|
Assignee | |
Comment 1•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/34111/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/34111/
Attachment #8717280 -
Flags: review?(MattN+bmo)
|
|
Assignee | |
Updated•8 years ago
|
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
|
||
Updated•8 years ago
|
Attachment #8717280 -
Flags: review?(MattN+bmo) → review+
|
|
||
Comment 2•8 years ago
|
||
Comment on attachment 8717280 [details] MozReview Request: Bug 1246843 - Add a default page for mozscreenshots mochitest;r=MattN https://reviewboard.mozilla.org/r/34111/#review30805
|
|
Assignee | |
Comment 3•8 years ago
|
||
Comment on attachment 8717280 [details] MozReview Request: Bug 1246843 - Add a default page for mozscreenshots mochitest;r=MattN Review request updated; see interdiff: https://reviewboard.mozilla.org/r/34111/diff/1-2/
|
|
Assignee | |
Comment 4•8 years ago
|
||
Last version caused a leak due to loading about:home before TestRunner.start set preferences. The new version loads the tab inside of TestRunner.start. Here are some pushes for comparison (shouldn't change results in non-devtools runs I think): Base: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d053037677b8&exclusion_profile=false&filter-tier=1&filter-tier=2&filter-tier=3 New: https://treeherder.mozilla.org/#/jobs?repo=try&revision=94899d85ee71&exclusion_profile=false&filter-tier=1&filter-tier=2&filter-tier=3
|
|
Assignee | |
Updated•8 years ago
|
Attachment #8717280 -
Attachment description: MozReview Request: Bug 1246843 - Set default page in mozscreenshots mochitest as about:home;r=MattN → MozReview Request: Bug 1246843 - Add a default page for mozscreenshots mochitest;r=MattN
|
|
Assignee | |
Comment 5•8 years ago
|
||
Comment on attachment 8717280 [details] MozReview Request: Bug 1246843 - Add a default page for mozscreenshots mochitest;r=MattN Review request updated; see interdiff: https://reviewboard.mozilla.org/r/34111/diff/2-3/
|
|
Assignee | |
Updated•8 years ago
|
Attachment #8717280 -
Flags: review+ → review?(MattN+bmo)
|
|
Assignee | |
Comment 6•8 years ago
|
||
There was still a leak - seemed to have something to do with unloading about:home and re-adding it for the tabs test. So instead we discussed bundling a home page with the addon - this is it. Base: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2214bf9e74aa&exclusion_profile=false&filter-tier=1&filter-tier=2&filter-tier=3 New: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a86a2fb6b112&exclusion_profile=false&filter-tier=1&filter-tier=2&filter-tier=3
|
|
Assignee | |
Updated•8 years ago
|
Summary: Set default page in mozscreenshots mochitest to be about:home → Set a default page in mozscreenshots mochitest
|
|
||
Updated•8 years ago
|
Attachment #8717280 -
Flags: review?(MattN+bmo) → review+
|
|
||
Comment 7•8 years ago
|
||
Comment on attachment 8717280 [details] MozReview Request: Bug 1246843 - Add a default page for mozscreenshots mochitest;r=MattN https://reviewboard.mozilla.org/r/34111/#review30987 Thanks ::: browser/tools/mozscreenshots/mozscreenshots/extension/TestRunner.jsm:11 (Diff revision 3) > const env = Cc["@mozilla.org/process/environment;1"].getService(Ci.nsIEnvironment); > > +Cu.import("resource://testing-common/BrowserTestUtils.jsm"); > Cu.import("resource://gre/modules/FileUtils.jsm"); > Cu.import("resource://gre/modules/Services.jsm"); > Cu.import("resource://gre/modules/Task.jsm"); > Cu.import("resource://gre/modules/Timer.jsm"); > Cu.import("resource://gre/modules/XPCOMUtils.jsm"); > Cu.import("resource://gre/modules/osfile.jsm"); > > Cu.import("chrome://mozscreenshots/content/Screenshot.jsm"); > > +const HOME_PAGE = "chrome://mozscreenshots/content/lib/mozscreenshots.html"; Move this up with the other const ::: browser/tools/mozscreenshots/mozscreenshots/extension/lib/mozscreenshots.html:1 (Diff revision 3) > +<!doctype html> Nit: `<!DOCTYPE html>` I always see caps used for the first part. ::: browser/tools/mozscreenshots/mozscreenshots/extension/lib/mozscreenshots.html:1 (Diff revision 3) > +<!doctype html> > +<html> Add the MPL license header to the three new text-based files
|
||
Comment 9•8 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/337331f99088
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
|
|
||
Updated•7 years ago
|
Component: General → mozscreenshots
Product: Firefox → Testing
Target Milestone: Firefox 47 → ---
You need to log in
before you can comment on or make changes to this bug.
Description
•