This (finally!) merges GitHub master. Changelog: https://github.com/mozilla-services/screenshots/blob/master/CHANGELOG.md#version-1600 Note that we've been using one version number for the server and add-on, so versions 11-15 have been server-only releases. (We'll separate the two eventually, but haven't yet.) The changes aren't that easy to copy over, so it would be best to view them on the link, but for the record I'll put them down here: Version 16.0.0 * Fix tests failing when run against photon-y Firefox without MOZ_PHOTON_THEME defined [beec56b](https://github.com/mozilla-services/screenshots/commit/beec56b) * Deal with lack of MOZ_PHOTON_THEME on 57+ [3e9eba3](https://github.com/mozilla-services/screenshots/commit/3e9eba3) * Make run-addon work with new legacy pref. Fixes [#3333](https://github.com/mozilla-services/screenshots/issues/3333) [b9a776b](https://github.com/mozilla-services/screenshots/commit/b9a776b) * Immediately exit when Firefox is exiting. Fixes [#3323](https://github.com/mozilla-services/screenshots/issues/3323) [916c353](https://github.com/mozilla-services/screenshots/commit/916c353) * Fix tests, enable legacy extensions via pref during tests ([#3324](https://github.com/mozilla-services/screenshots/issues/3324)) [c60fd37](https://github.com/mozilla-services/screenshots/commit/c60fd37) * Make tests resilient to a browserAction or pageAction ([#3317](https://github.com/mozilla-services/screenshots/issues/3317)) Fixes [#3306](https://github.com/mozilla-services/screenshots/issues/3306) [3c15014](https://github.com/mozilla-services/screenshots/commit/3c15014) * Replace the WebExtension browser action with a Photon page action. ([#3239](https://github.com/mozilla-services/screenshots/issues/3239)) * Replace the WebExtension browser action with a Photon page action. This removes the browser action and adds a Photon page action, pursuant to https://bugzilla.mozilla.org/show_bug.cgi?id=1366041. Right now Photon page actions are unrelated to WebExtension page actions unfortunately, which means that this patch has to do most of its work in bootstrap.js. The WebExtension part passes messages to bootstrap.js to handle clicks and update the action's title and icon as necessary. * Test fix: Replace the WebExtension browser action with a Photon page action. Fix the test for the previous commit. * Update SHOOTER_BUTTON_ID in test.js. * Use the Photon page action when supported, the WebExtension browser action when not. Extend the Photon-related port used between bootstrap.js and the WebExtension so that bootstrap.js can tell the WebExtension whether it's OK to use the Photon page action. The WebExtension should not attempt to use the browser action when Photon is enabled because bootstrap.js will have removed the browser action's navbar button. Modify the test so that it checks the Photon page action when Photon is enabled and the browser action when it's not. * Fix the expected button label in test.js for the Photon page action. * Maybe fix PageActions eslint errors. [4396250](https://github.com/mozilla-services/screenshots/commit/4396250) * remove high DPI shot capture for full page [fde181d](https://github.com/mozilla-services/screenshots/commit/fde181d) * Sets background highlight width to 100% [aee88eb](https://github.com/mozilla-services/screenshots/commit/aee88eb) * Remove duplicate drawWindow call for shot preview [2a5dd27](https://github.com/mozilla-services/screenshots/commit/2a5dd27) * Update shot preview save icon [735b3f9](https://github.com/mozilla-services/screenshots/commit/735b3f9) * Add reason to webExtension.startup and shutdown Will be useful when https://bugzilla.mozilla.org/show_bug.cgi?id=1372750 and https://bugzilla.mozilla.org/show_bug.cgi?id=1373749 are fixed [d97d4a7](https://github.com/mozilla-services/screenshots/commit/d97d4a7) * Protect against an empty IPC response ([#3037](https://github.com/mozilla-services/screenshots/issues/3037))This happened in some weird corner case while debugging [14df39d](https://github.com/mozilla-services/screenshots/commit/14df39d) * Guard access of this.save when un-disabling the save button ([#3030](https://github.com/mozilla-services/screenshots/issues/3030)). This can happen after the worker has been torn down, and this.save isn't defined [cf6e72a](https://github.com/mozilla-services/screenshots/commit/cf6e72a) * Set dimensions for icon and add to startup ([#3136](https://github.com/mozilla-services/screenshots/issues/3136)) [9959ede](https://github.com/mozilla-services/screenshots/commit/9959ede) * Disable Screenshots in private windows. Fixes [#3120](https://github.com/mozilla-services/screenshots/issues/3120) [aefc639](https://github.com/mozilla-services/screenshots/commit/aefc639) * about:home is not treated like about:newtab ([#3088](https://github.com/mozilla-services/screenshots/issues/3088)). Fixes [#3029](https://github.com/mozilla-services/screenshots/issues/3029) [4633694](https://github.com/mozilla-services/screenshots/commit/4633694) * preview shot before saving full page/visibleremove addToMyShots [c87db61](https://github.com/mozilla-services/screenshots/commit/c87db61)

Note SoftVision/Cosmin has QA'd a Nightly build with this version of the add-on. At some point we should talk about the review process for ongoing updates – it's not easy to review a big patch like this, but then it wouldn't be easy to review a dozen smaller patches either.

Comment on attachment 8897950 [details] Bug 1390985 - Export Screenshots 16.1.0 to Firefox https://reviewboard.mozilla.org/r/169250/#review175520

Comment on attachment 8897951 [details] Bug 1390985 - Export Screenshots 16.0.0 to Firefox (all code changes) https://reviewboard.mozilla.org/r/169252/#review176970 r=me with follow-ups filed for CSS-based handling of icon selection and browserAction hiding, and the rest of the issues fixed. ::: browser/extensions/screenshots/bootstrap.js:74 (Diff revision 1) > + ITEM_ID: "appMenu-library-screenshots", > + > + init(webExtension) { > + this._initialized = true; > + let permissionPages = [...webExtension.extension.permissions].filter(p => (/^https?:\/\//i).test(p)); > + this.PAGE_TO_OPEN = permissionPages.length ? permissionPages[0].replace(/\*$/, "") : "https://screenshots.firefox.com/"; You shouldn't expect the ordering of host permissions to be deterministic, so if you're expecting only one host, please check `length == 1` ::: browser/extensions/screenshots/bootstrap.js:95 (Diff revision 1) > + let item = win.document.getElementById(this.ITEM_ID); > + if (item) { > + item.remove(); > + } > + } > + CustomizableUI.removeListener(this); Please set `this._initialized` to false here. ::: browser/extensions/screenshots/bootstrap.js:111 (Diff revision 1) > + let {nextSibling} = libraryViewInsertionPoint; > + let item = win.document.createElement("toolbarbutton"); > + item.className = "subviewbutton subviewbutton-iconic"; > + item.addEventListener("command", () => win.openUILinkIn(this.PAGE_TO_OPEN, "tab")); > + item.id = this.ITEM_ID; > + let iconURL = win.devicePixelRatio >= 1.1 ? this.ICON_URL_2X : this.ICON_URL; `devicePixelRatio` can change as screens are attached or removed, or windows are moved between them. This should use CSS to determine the correct image. ::: browser/extensions/screenshots/bootstrap.js:244 (Diff revision 1) > + > + let id = "screenshots"; > + let port = null; > + let baseIconPath = addonResourceURI.spec + "webextension/"; > + > + let {Management: {global: {tabTracker}}} = Cu.import("resource://gre/modules/Extension.jsm", {}); Please don't reach into extension modules like this. Use the `tabManager` property of your extension object instead. ::: browser/extensions/screenshots/bootstrap.js:271 (Diff revision 1) > + CustomizableUI.destroyWidget(cuiWidgetID); > + CustomizableUI.removeListener(this); This seems super dodgy. If you're going to leave the browserAction registered and hide it, I'd rather you just hide it with CSS. Or, even better, just switch to a pageAction and have the extension never show it when it's not needed.

Comment on attachment 8897951 [details] Bug 1390985 - Export Screenshots 16.0.0 to Firefox (all code changes) https://reviewboard.mozilla.org/r/169252/#review176970 > You shouldn't expect the ordering of host permissions to be deterministic, so if you're expecting only one host, please check `length == 1` Warning added [here](https://github.com/mozilla-services/screenshots/pull/3408/files#diff-4b80223300be1233ce41b6b9d0c08761L74) > Please set `this._initialized` to false here. Fixed [here](https://github.com/mozilla-services/screenshots/pull/3408/files#diff-4b80223300be1233ce41b6b9d0c08761R99) > Please don't reach into extension modules like this. Use the `tabManager` property of your extension object instead. Fixed [here](https://github.com/mozilla-services/screenshots/pull/3408/files#diff-4b80223300be1233ce41b6b9d0c08761L244)

Comment on attachment 8897950 [details] Bug 1390985 - Export Screenshots 16.1.0 to Firefox Comments have followup issues or fixes per discussion.

Comment on attachment 8897950 [details] Bug 1390985 - Export Screenshots 16.1.0 to Firefox https://reviewboard.mozilla.org/r/169250/#review178858

Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/7acf5f94b0ad Export Screenshots 16.1.0 to Firefox r=kmag