Version 18.0.0 exported in Bug 1396060 didn't stick, so here's another try with version 19.0.0. Changes in 19.0.0: Changelog: https://github.com/mozilla-services/screenshots/blob/master/CHANGELOG.md#version-1900 * Remove Photon-related conditionals in tests (https://github.com/mozilla-services/screenshots/commit/0b31d58) * remove browserAction. This changes the add-on to require the Photon page action, with no fallback to a browserAction - Removes test for Photon (assumes it is present) - Removes bootstrap.js code that deletes the browserAction button - Removes webextension code references to browserAction - Removes Photon conditionals (i.e., assume it's always Photon) - Make 57a1 the minimum version for the webextension/install.rdf. Fixes https://github.com/mozilla-services/screenshots/issues/3468 (https://github.com/mozilla-services/screenshots/commit/d4c56ae) * Add setting to control binary or base64 upload. This adds `$SCREENSHOTS_UPLOAD_BINARY=true` to turn the feature on. Fixes https://github.com/mozilla-services/screenshots/issues/3481 (https://github.com/mozilla-services/screenshots/commit/92b0d53) * Convert to semantic locale strings for the slides. The numeric locale ids have made reordering complicated (https://github.com/mozilla-services/screenshots/commit/3d590fb) * Avoid form uploads from being truncated. `FormData` was not creating correct request bodies for large images. This changes the code to manually construct the form upload. Fixes https://github.com/mozilla-services/screenshots/issues/3472 (https://github.com/mozilla-services/screenshots/commit/671b003) * Put a guard around the exception stack rewriting (https://github.com/mozilla-services/screenshots/commit/dbc4750) * Revert "Remove the full page and save visible buttons from onboarding". This reverts commit 1887c38903ce91199f389a345095d6a0546004ac. (https://github.com/mozilla-services/screenshots/commit/33bb5ff) * Add a new slide to the tour. This slide includes the pageAction interface instead of the old toolbar button. Fixes https://github.com/mozilla-services/screenshots/issues/3442 (https://github.com/mozilla-services/screenshots/commit/9f072c0) (https://github.com/mozilla-services/screenshots/commit/2550449) * Change slide image to match Photon-style in browser image. Fixes https://github.com/mozilla-services/screenshots/issues/3443 (https://github.com/mozilla-services/screenshots/commit/dbad266) Changes from 18.0.0: * Run all add-on svg files through svgo (https://github.com/mozilla-services/screenshots/commit/40b9fe0) * Fix icon appearance for Photon page action. See Bug 1395284]. Right now, the icon is too dark, so it doesn't match the appearance of the other Photon page actions. The problem is that the URI passed as the action's iconURL is a `file://` URI. The Photon theme uses context-fill and context-fill-opacity in SVG in order to style SVG icons correctly, and SVG context painting is not supported for file `bootstrap.js` should pass a `moz-extension://` URI instead, which context painting does support, and which is what the WebExtension browser action toolbar button uses. Additionally, the icon SVG used by the Photon page action needs to be updated with fill-opacity="context-fill-opacity". (https://github.com/mozilla-services/screenshots/commit/b246cb9) * Add logging of unexpected clipboard state (https://github.com/mozilla-services/screenshots/issues/3430). This logs cases when the passed-in text is empty, or the textarea select doesn't appear to work. Logs are sent to Sentry. Fixes https://github.com/mozilla-services/screenshots/issues/3406 (https://github.com/mozilla-services/screenshots/commit/10b7c0f) * Fixed next and prev buttons for rtl (https://github.com/mozilla-services/screenshots/commit/5a08464) * Moved Save/Cancel buttons from right to left for rtl languages (https://github.com/mozilla-services/screenshots/issues/3412) (https://github.com/mozilla-services/screenshots/commit/115d6ed) * Send image to server in binary (https://github.com/mozilla-services/screenshots/commit/9c558ce)

Try build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=eac1b6ccdc1f7f3f970bf9005b38968b189d6c6d

QA reviewed this, they found #3491 (https://github.com/mozilla-services/screenshots/issues/3491) - Screenshots works in Private Browsing Mode, if it's used from Nightly "Page Actions" menu - normal Not that bug exists in the current 16.0.0 as well.

Comment on attachment 8907779 [details] Bug 1399615 - Export Screenshots 19.0.0 to Firefox https://reviewboard.mozilla.org/r/179460/#review185216 r=me for everything but the form data encoding. I might be able to be convinced that it's acceptable, but it would take a lot. ::: browser/extensions/screenshots/bootstrap.js:2 (Diff revision 1) > +// TODO: re-enable > +/* eslint-disable */ Why do you need to disable ESLint? ::: browser/extensions/screenshots/test/browser/.eslintrc.yml:9 (Diff revision 1) > + no-unused-vars: off > + no-undef: off Why do you need to disable these rules? ::: browser/extensions/screenshots/webextension/background/takeshot.js:159 (Diff revision 1) > + /** Creates a multipart TypedArray, given {name: value} fields > + and {name: blob} files > + > + Returns {body, "content-type"} > + */ > + function createMultipart(fields, fileField, fileFilename, blob) { > + let boundary = "---------------------------ScreenshotBoundary" + Date.now(); > + return blobToArray(blob).then((blobAsBuffer) => { > + let body = []; > + for (let name in fields) { > + body.push("--" + boundary); > + body.push(`Content-Disposition: form-data; name="${name}"`); > + body.push(""); > + body.push(fields[name]); > + } > + body.push("--" + boundary); > + body.push(`Content-Disposition: form-data; name="${fileField}"; filename="${fileFilename}"`); > + body.push(`Content-Type: ${blob.type}`); > + body.push(""); > + body.push(""); > + body = body.join("\r\n"); > + let enc = new TextEncoder("utf-8"); > + body = enc.encode(body); > + body = concatBuffers(body.buffer, blobAsBuffer); > + let tail = "\r\n" + "--" + boundary + "--"; > + tail = enc.encode(tail); > + body = concatBuffers(body, tail.buffer); > + return { > + "content-type": `multipart/form-data; boundary=${boundary}`, > + body > + }; > + }); > + } You should be using a FormData object for this. Aside from being considerably simpler, it's also considerably more efficient and reliable.

Comment on attachment 8907779 [details] Bug 1399615 - Export Screenshots 19.0.0 to Firefox https://reviewboard.mozilla.org/r/179460/#review185454 ::: browser/extensions/screenshots/bootstrap.js:2 (Diff revision 1) > +// TODO: re-enable > +/* eslint-disable */ We have [a bug](https://github.com/mozilla-services/screenshots/issues/3450) to put this back, but at the moment the Mozilla eslint plugin is broken (see [Bug 1395879](https://bugzilla.mozilla.org/show_bug.cgi?id=1395879) Since we're not doing much work on this file it seemed easiest to disable it for now and wait for the upstream fix. ::: browser/extensions/screenshots/webextension/background/takeshot.js:159 (Diff revision 1) > + /** Creates a multipart TypedArray, given {name: value} fields > + and {name: blob} files > + > + Returns {body, "content-type"} > + */ > + function createMultipart(fields, fileField, fileFilename, blob) { > + let boundary = "---------------------------ScreenshotBoundary" + Date.now(); > + return blobToArray(blob).then((blobAsBuffer) => { > + let body = []; > + for (let name in fields) { > + body.push("--" + boundary); > + body.push(`Content-Disposition: form-data; name="${name}"`); > + body.push(""); > + body.push(fields[name]); > + } > + body.push("--" + boundary); > + body.push(`Content-Disposition: form-data; name="${fileField}"; filename="${fileFilename}"`); > + body.push(`Content-Type: ${blob.type}`); > + body.push(""); > + body.push(""); > + body = body.join("\r\n"); > + let enc = new TextEncoder("utf-8"); > + body = enc.encode(body); > + body = concatBuffers(body.buffer, blobAsBuffer); > + let tail = "\r\n" + "--" + boundary + "--"; > + tail = enc.encode(tail); > + body = concatBuffers(body, tail.buffer); > + return { > + "content-type": `multipart/form-data; boundary=${boundary}`, > + body > + }; > + }); > + } Using FormData specifically broke for large uploads, the server wasn't being sent the entire payload. I should probably open a bug for that on the Firefox side; our bug was [#3472](https://github.com/mozilla-services/screenshots/issues/3472). Request bodies were being truncated at around 1200 bytes. While the manual body construction fixed it, I didn't feel great about that solution, so the entire form upload portion is pref'd off for now (the setting is in buildSettings.js), and so this version of the add-on will still send the images as data URLs.

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

Backed out for failing browser-chrome's browser/components/uitour/test/browser_UITour_availableTargets.js, at least on OS X: https://hg.mozilla.org/integration/autoland/rev/3cb1ce131f7cab332f036cecd59e5d104c981fa8 Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=c56a205100ac8747670ad5ad5ba1d6c66ce77310&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=131323634&repo=autoland 10:39:36 INFO - 936 INFO TEST-START | browser/components/uitour/test/browser_UITour_availableTargets.js 10:39:42 INFO - TEST-INFO | started process screencapture 10:39:42 INFO - TEST-INFO | screencapture: exit 0 10:39:42 INFO - Buffered messages logged at 10:39:36 10:39:42 INFO - 937 INFO Entering test bound setup_UITourTest 10:39:42 INFO - 938 INFO Leaving test bound setup_UITourTest 10:39:42 INFO - 939 INFO Entering test bound test_availableTargets 10:39:42 INFO - Buffered messages finished 10:39:42 ERROR - 940 INFO TEST-UNEXPECTED-FAIL | browser/components/uitour/test/browser_UITour_availableTargets.js | Should enable Screenshots - timed out after 50 tries. - 10:39:42 INFO - Stack trace: 10:39:42 INFO - chrome://mochitests/content/browser/browser/components/uitour/test/head.js:add_UITour_task/genFun/</</</funcPromise<:450

Please verify that browser/extensions/screenshots/test/browser/browser_screenshots_ui_check.js is also fixed: https://treeherder.mozilla.org/logviewer.html#?job_id=131324151&repo=autoland

(In reply to Ian Bicking (:ianbicking) from comment #5) > Using FormData specifically broke for large uploads, the server wasn't being > sent the entire payload. I should probably open a bug for that on the > Firefox side; our bug was > [#3472](https://github.com/mozilla-services/screenshots/issues/3472). > Request bodies were being truncated at around 1200 bytes. Yes, please file a Firefox (/Necko) bug for this and CC me.

I'll take a look at the UITour test failure, clearing the needinfo on Ian.

Sorry about those tests, I was a overfocused on our CI tests. Both tests are fixed in the revised patch.

Comment on attachment 8907779 [details] Bug 1399615 - Export Screenshots 19.0.0 to Firefox https://reviewboard.mozilla.org/r/179460/#review185630

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 0e744eeacd74 -d 008c2a926077: rebasing 420911:0e744eeacd74 "Bug 1399615 - Export Screenshots 19.0.0 to Firefox r=kmag" (tip) merging browser/extensions/screenshots/bootstrap.js merging browser/extensions/screenshots/install.rdf merging browser/extensions/screenshots/moz.build merging browser/extensions/screenshots/test/browser/browser_screenshots_ui_check.js merging browser/extensions/screenshots/test/browser/head.js merging browser/extensions/screenshots/webextension/_locales/ach/messages.json merging browser/extensions/screenshots/webextension/_locales/ar/messages.json merging browser/extensions/screenshots/webextension/_locales/ast/messages.json merging browser/extensions/screenshots/webextension/_locales/az/messages.json merging browser/extensions/screenshots/webextension/_locales/be/messages.json merging browser/extensions/screenshots/webextension/_locales/bg/messages.json merging browser/extensions/screenshots/webextension/_locales/bn_BD/messages.json merging browser/extensions/screenshots/webextension/_locales/ca/messages.json merging browser/extensions/screenshots/webextension/_locales/cak/messages.json merging browser/extensions/screenshots/webextension/_locales/cs/messages.json merging browser/extensions/screenshots/webextension/_locales/cy/messages.json merging browser/extensions/screenshots/webextension/_locales/da/messages.json merging browser/extensions/screenshots/webextension/_locales/de/messages.json merging browser/extensions/screenshots/webextension/_locales/dsb/messages.json merging browser/extensions/screenshots/webextension/_locales/el/messages.json merging browser/extensions/screenshots/webextension/_locales/en_GB/messages.json merging browser/extensions/screenshots/webextension/_locales/en_US/messages.json merging browser/extensions/screenshots/webextension/_locales/eo/messages.json merging browser/extensions/screenshots/webextension/_locales/es_AR/messages.json merging browser/extensions/screenshots/webextension/_locales/es_CL/messages.json merging browser/extensions/screenshots/webextension/_locales/es_ES/messages.json merging browser/extensions/screenshots/webextension/_locales/es_MX/messages.json merging browser/extensions/screenshots/webextension/_locales/et/messages.json merging browser/extensions/screenshots/webextension/_locales/fa/messages.json merging browser/extensions/screenshots/webextension/_locales/fi/messages.json merging browser/extensions/screenshots/webextension/_locales/fr/messages.json merging browser/extensions/screenshots/webextension/_locales/fy_NL/messages.json merging browser/extensions/screenshots/webextension/_locales/ga_IE/messages.json merging browser/extensions/screenshots/webextension/_locales/gu_IN/messages.json merging browser/extensions/screenshots/webextension/_locales/he/messages.json merging browser/extensions/screenshots/webextension/_locales/hi_IN/messages.json merging browser/extensions/screenshots/webextension/_locales/hr/messages.json merging browser/extensions/screenshots/webextension/_locales/hsb/messages.json merging browser/extensions/screenshots/webextension/_locales/hu/messages.json merging browser/extensions/screenshots/webextension/_locales/hy_AM/messages.json merging browser/extensions/screenshots/webextension/_locales/id/messages.json merging browser/extensions/screenshots/webextension/_locales/it/messages.json merging browser/extensions/screenshots/webextension/_locales/ja/messages.json merging browser/extensions/screenshots/webextension/_locales/ka/messages.json merging browser/extensions/screenshots/webextension/_locales/kab/messages.json merging browser/extensions/screenshots/webextension/_locales/kk/messages.json merging browser/extensions/screenshots/webextension/_locales/ko/messages.json merging browser/extensions/screenshots/webextension/_locales/lij/messages.json merging browser/extensions/screenshots/webextension/_locales/lo/messages.json merging browser/extensions/screenshots/webextension/_locales/lt/messages.json merging browser/extensions/screenshots/webextension/_locales/mk/messages.json merging browser/extensions/screenshots/webextension/_locales/mr/messages.json merging browser/extensions/screenshots/webextension/_locales/ms/messages.json merging browser/extensions/screenshots/webextension/_locales/my/messages.json merging browser/extensions/screenshots/webextension/_locales/nb_NO/messages.json merging browser/extensions/screenshots/webextension/_locales/nl/messages.json merging browser/extensions/screenshots/webextension/_locales/nn_NO/messages.json merging browser/extensions/screenshots/webextension/_locales/pl/messages.json merging browser/extensions/screenshots/webextension/_locales/pt_BR/messages.json merging browser/extensions/screenshots/webextension/_locales/pt_PT/messages.json merging browser/extensions/screenshots/webextension/_locales/rm/messages.json merging browser/extensions/screenshots/webextension/_locales/ro/messages.json merging browser/extensions/screenshots/webextension/_locales/ru/messages.json merging browser/extensions/screenshots/webextension/_locales/sk/messages.json merging browser/extensions/screenshots/webextension/_locales/sl/messages.json merging browser/extensions/screenshots/webextension/_locales/sq/messages.json merging browser/extensions/screenshots/webextension/_locales/sr/messages.json merging browser/extensions/screenshots/webextension/_locales/sv_SE/messages.json merging browser/extensions/screenshots/webextension/_locales/ta/messages.json merging browser/extensions/screenshots/webextension/_locales/te/messages.json merging browser/extensions/screenshots/webextension/_locales/th/messages.json merging browser/extensions/screenshots/webextension/_locales/tl/messages.json merging browser/extensions/screenshots/webextension/_locales/tr/messages.json merging browser/extensions/screenshots/webextension/_locales/uk/messages.json merging browser/extensions/screenshots/webextension/_locales/ur/messages.json merging browser/extensions/screenshots/webextension/_locales/vi/messages.json merging browser/extensions/screenshots/webextension/_locales/zh_CN/messages.json merging browser/extensions/screenshots/webextension/_locales/zh_TW/messages.json merging browser/extensions/screenshots/webextension/background/main.js merging browser/extensions/screenshots/webextension/background/senderror.js merging browser/extensions/screenshots/webextension/background/startBackground.js merging browser/extensions/screenshots/webextension/background/takeshot.js merging browser/extensions/screenshots/webextension/build/buildSettings.js merging browser/extensions/screenshots/webextension/build/inlineSelectionCss.js merging browser/extensions/screenshots/webextension/build/onboardingCss.js merging browser/extensions/screenshots/webextension/build/onboardingHtml.js merging browser/extensions/screenshots/webextension/build/shot.js merging browser/extensions/screenshots/webextension/clipboard.js merging browser/extensions/screenshots/webextension/icons/back-highlight.svg merging browser/extensions/screenshots/webextension/icons/back.svg merging browser/extensions/screenshots/webextension/icons/cancel.svg merging browser/extensions/screenshots/webextension/icons/cloud.svg merging browser/extensions/screenshots/webextension/icons/done.svg merging browser/extensions/screenshots/webextension/icons/download.svg merging browser/extensions/screenshots/webextension/icons/icon-16-v2.svg merging browser/extensions/screenshots/webextension/icons/icon-32-v2.svg merging browser/extensions/screenshots/webextension/icons/icon-highlight-32-v2.svg merging browser/extensions/screenshots/webextension/icons/icon-starred-32-v2.svg merging browser/extensions/screenshots/webextension/icons/icon-welcome-face-without-eyes.svg merging browser/extensions/screenshots/webextension/icons/menu-fullpage.svg merging browser/extensions/screenshots/webextension/icons/menu-myshot-white.svg merging browser/extensions/screenshots/webextension/icons/menu-myshot.svg merging browser/extensions/screenshots/webextension/icons/menu-visible.svg merging browser/extensions/screenshots/webextension/icons/onboarding-2.png merging browser/extensions/screenshots/webextension/icons/onboarding-3.png merging browser/extensions/screenshots/webextension/icons/onboarding-4.png merging browser/extensions/screenshots/webextension/manifest.json merging browser/extensions/screenshots/webextension/onboarding/slides.html merging browser/extensions/screenshots/webextension/selector/shooter.js warning: /repos/mozreview-gecko/browser/extensions/screenshots/webextension/icons/onboarding-2.png looks like a binary file. warning: conflicts while merging browser/extensions/screenshots/webextension/icons/onboarding-2.png! (edit, then use 'hg resolve --mark') warning: /repos/mozreview-gecko/browser/extensions/screenshots/webextension/icons/onboarding-3.png looks like a binary file. warning: conflicts while merging browser/extensions/screenshots/webextension/icons/onboarding-3.png! (edit, then use 'hg resolve --mark') warning: /repos/mozreview-gecko/browser/extensions/screenshots/webextension/icons/onboarding-4.png looks like a binary file. warning: conflicts while merging browser/extensions/screenshots/webextension/icons/onboarding-4.png! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)

Clearing the needinfo on Ian since it doesn't look like it's necessary anymore. Apologies if I'm wrong.