1402932 - Update Screenshots to version 19.1.0 (JPEG support)

Status: VERIFIED FIXED

(Firefox :: Screenshots, defect)

Description

[Ian Bicking (:ianbicking)]

We're going to be doing some smaller patches with the intention to uplift to 57.

This export adds support for generating JPEGs. For some Full Page captures (especially of image-heavy pages) the size of the screenshot files is very large and overall performance is problematic. By using JPEGs for large sites we avoid these cases.

Commits/changelog:

* Bug fix for https://github.com/mozilla-services/screenshots/issues/3513, create proper extension for downloads (https://github.com/mozilla-services/screenshots/issues/3543)
  The clip object is used to inform the download filename, so we need to add a clip before generating the filename (https://github.com/mozilla-services/screenshots/commit/145a456)
* Remove bad console.log statement (https://github.com/mozilla-services/screenshots/commit/200f93d)
* Upload jpeg https://github.com/mozilla-services/screenshots/issues/3513
  - Start https://github.com/mozilla-services/screenshots/issues/220 , allow JPEG uploads, and respect content-type for JPEG or PNG
  - use JPEG for large shots
  - Allows JPEGs on the server, both to pass content checks, and to make use of stored content-types (instead of assuming image/png).
  - Puts an clip.image.type into shot objects
  - Uses .jpg for filenames when appropriate
  - Adds a new buildSetting for controlling the cutoff when we use JPEG
  - If a PNG image is too large, tries to make a JPEG and substitutes if the JPEG is actually smaller
  - Refactor some data:-URL and blob converstion functions into their own module.
  - https://github.com/mozilla-services/screenshots/commit/78e0717

Comment 1

[Ian Bicking (:ianbicking)]

Attachment: Bug 1402932 - Export Screenshots 19.1.0 to Firefox

Review commit: https://reviewboard.mozilla.org/r/183368/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/183368/

Comment 2

[Kris Maglione [:kmag]]

Comment on attachment 8911956 [details]
Bug 1402932 - Export Screenshots 19.1.0 to Firefox

https://reviewboard.mozilla.org/r/183368/#review188482

::: browser/extensions/screenshots/webextension/blobConverters.js:16
(Diff revision 1)
> +    const blob = new Blob([data], {type: contentType});
> +    return blob;
> +  };
> +
> +  exports.getTypeFromDataUrl = function(url) {
> +    let contentType = url.split(',')[0];

Please add a limit argument to `url.split()`, or you're going to wind up creating an unused string with the URL data, which could be huge.

::: browser/extensions/screenshots/webextension/selector/shooter.js:71
(Diff revision 1)
> -    return canvas.toDataURL();
> +    let limit = buildSettings.pngToJpegCutoff;
> +    let dataUrl = canvas.toDataURL();
> +    if (limit && dataUrl.length > limit) {
> +      let jpegDataUrl = canvas.toDataURL("image/jpeg");
> +      if (jpegDataUrl.length < dataUrl.length) {
> +        // Only use the JPEG if it is actually smaller

Is it ever not actually smaller?

Comment 3

[Ian Bicking (:ianbicking)]

Comment on attachment 8911956 [details]
Bug 1402932 - Export Screenshots 19.1.0 to Firefox

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183368/diff/1-2/

Comment 4

[Ian Bicking (:ianbicking)]

Comment on attachment 8911956 [details]
Bug 1402932 - Export Screenshots 19.1.0 to Firefox

https://reviewboard.mozilla.org/r/183368/#review188482

> Is it ever not actually smaller?

Yes, on text-heavy pages (e.g., Wikipedia pages) PNG often does better than JPEG.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ea48aa0eb4dc
Export Screenshots 19.1.0 to Firefox r=kmag

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/ea48aa0eb4dc

Comment 7

[Ian Bicking (:ianbicking)]

Comment on attachment 8911956 [details]
Bug 1402932 - Export Screenshots 19.1.0 to Firefox

Approval Request Comment
[Feature/Bug causing the regression]:
Screenshots performs very badly for some large (5Mb+) images (badly enough we cut the feature from the 56 version of Screenshots). By using JPEGs we avoid creating large images.

[User impact if declined]:
Users who use Screenshots Full Page on image-heavy pages will see very poor performance.


[Is this code covered by automated tests?]:
Partially

[Has the fix been verified in Nightly?]:
No (the 2017-09-26 Nightly still contains 19.0.0)
I will forward this to the testpilot-qa team to verify with tomorrow's build.

[Needs manual test from QE? If yes, steps to reproduce]: 
No, but would be okay to verify:
1. Go to imgur.com
2. Start Screenshots
3. Choose Full Page
4. Download or upload; if you download you should get a .jpg file, if you upload it will upload in a reasonable amount of time

[List of other uplifts needed for the feature/fix]:
None

[Is the change risky?]:
I don't believe it is risky. I'm confident the risk is contained to the Screenshots experience itself, and won't affect Firefox generally.

[Why is the change risky/not risky?]:
Code change is contained to the WebExtension.

[String changes made/needed]:
No changes

Comment 8

[Cosmin Muntean [:cmuntean], Ecosystem QA]

I have tested this on latest Nightly (58.0a1, Build ID: 20170927100120) with Screenshots 19.1.0. 
In order to test it I have used the provided STR from comment 7 on multiple large pages (eg: imgur.com, giphy.com, cnn.com and long articles from wikipedia) as well as several other scenarios. 
While testing I haven't found any new issues, the large shots are correctly saved with ".jpg" extension and the lag is significantly reduced when uploading a large shot versus the older Screenshots version. 

Tested on Windows 10x64, Windows 8.1 x32, Windows 7x64, Mac Os 10.12 and also on Ubuntu 14.04.

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8911956 [details]
Bug 1402932 - Export Screenshots 19.1.0 to Firefox

Improve screenshot, taking it.
Should be in 57b4

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ca781d13823b

Comment 11

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

Investigated this on 57.0b12 (20171026211016) build, using Windows 10 x64, Ubuntu 16.04 x64, macOS 10.13 and different heavy content websites. The full page performance is considerably improved and also, the .jpg creation works properly. I can conclude that 57.0b12 build is verified fixed.