1465581 - Export Screenshots 33.0.0

Status: RESOLVED FIXED

(Firefox :: Screenshots, defect)

Description

[Barry Chen]

This is an export of Screenshots 32.2.0.

Changelog is at https://github.com/mozilla-services/screenshots/commit/daf6bbe30c0eedd5cf8475a706e55ca5cd3e8510.

Note that the 'mozillaAddons' permission change is being added in Bug 1465544.

### Add-on changes

* Update copy icon in notifications. ([#4485](https://github.com/mozilla-services/screenshots/issues/4485)) [7eecffc](https://github.com/mozilla-services/screenshots/commit/7eecffc)
* Update dependency raven-js to v3.25.2 ([#4428](https://github.com/mozilla-services/screenshots/issues/4428)) [e71b139](https://github.com/mozilla-services/screenshots/commit/e71b139)
* Ignore download canceled by user errors. Fixes [#3964](https://github.com/mozilla-services/screenshots/issues/3964) [9eb7f6c](https://github.com/mozilla-services/screenshots/commit/9eb7f6c)
* Avoid including blank scrollbar area in save-visible shots. Fixes [#4147](https://github.com/mozilla-services/screenshots/issues/4147) [e081c42](https://github.com/mozilla-services/screenshots/commit/e081c42)
* Make onboarding behaviour consistent ([#4342](https://github.com/mozilla-services/screenshots/issues/4342)). Fixes [#4183](https://github.com/mozilla-services/screenshots/issues/4183) [236e16d](https://github.com/mozilla-services/screenshots/commit/236e16d)
* Use WebExtension page action instead of Photon. ([#3756](https://github.com/mozilla-services/screenshots/issues/3756)) ([#3967](https://github.com/mozilla-services/screenshots/issues/3967)) [02b4c6e](https://github.com/mozilla-services/screenshots/commit/02b4c6e)
* Use precision select cursor ([#4361](https://github.com/mozilla-services/screenshots/issues/4361)). Fixes [#4187](https://github.com/mozilla-services/screenshots/issues/4187) [8208c0b](https://github.com/mozilla-services/screenshots/commit/8208c0b)

### Clean-up and Lint

* Clean up log.js [17341d4](https://github.com/mozilla-services/screenshots/commit/17341d4)
* Upgrade eslint-plugin-mozilla along with code fixes. ([#4322](https://github.com/mozilla-services/screenshots/issues/4322)) [ff9aa53](https://github.com/mozilla-services/screenshots/commit/ff9aa53)

### Localization

* Initialize en-CA content, copying over en-GB ([#4388](https://github.com/mozilla-services/screenshots/issues/4388)) [d818d86](https://github.com/mozilla-services/screenshots/commit/d818d86)

Comment 1

[Barry Chen]

The original version number in the description was incorrect: it should've been 33.0.0.  The patch will have the correct version number.

Comment 2

[Barry Chen]

Treeherder and Talos:
 - https://treeherder.mozilla.org/#/jobs?repo=try&revision=7d1a63765f384259f58bffdba705bd6c615c8cd2
 - https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=7d1a63765f384259f58bffdba705bd6c615c8cd2

Comment 3

[Barry Chen]

Attachment: Bug 1465581 - Export Screenshots 33.0.0 to Firefox (translations only);

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

Comment 4

[Barry Chen]

Attachment: Bug 1465581 - Export Screenshots 33.0.0 to Firefox (upgrade Raven to 3.25.2);

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

Comment 5

[Barry Chen]

Attachment: Bug 1465581 - Export Screenshots 33.0.0 to Firefox (code excluding translations and Raven update);

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

Comment 6

[Ian Bicking (:ianbicking)]

Comment on attachment 8982026 [details]
Bug 1465581 - Export Screenshots 33.0.0 to Firefox (upgrade Raven to 3.25.2);

https://reviewboard.mozilla.org/r/248078/#review254176

Comment 7

[Ian Bicking (:ianbicking)]

Comment on attachment 8982025 [details]
Bug 1465581 - Export Screenshots 33.0.0 to Firefox (translations only);

https://reviewboard.mozilla.org/r/248076/#review254178

Comment 8

[Ian Bicking (:ianbicking)]

Comment on attachment 8982027 [details]
Bug 1465581 - Export Screenshots 33.0.0 to Firefox (code excluding translations and Raven update);

https://reviewboard.mozilla.org/r/248080/#review254182

Comment 9

[Francesco Lodolo [:flod]]

Comment on attachment 8982025 [details]
Bug 1465581 - Export Screenshots 33.0.0 to Firefox (translations only);

https://reviewboard.mozilla.org/r/248076/#review254324

Comment 10

[Barry Chen]

Comment on attachment 8982025 [details]
Bug 1465581 - Export Screenshots 33.0.0 to Firefox (translations only);

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

Comment 11

[Barry Chen]

Comment on attachment 8982026 [details]
Bug 1465581 - Export Screenshots 33.0.0 to Firefox (upgrade Raven to 3.25.2);

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

Comment 12

[Barry Chen]

Comment on attachment 8982027 [details]
Bug 1465581 - Export Screenshots 33.0.0 to Firefox (code excluding translations and Raven update);

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

Comment 13

[Barry Chen]

try again without a duplicate file:
 - https://treeherder.mozilla.org/#/jobs?repo=try&revision=a1989802eb8f54afe0f14f694e29fc9833fae325
 - https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=a1989802eb8f54afe0f14f694e29fc9833fae325

Comment 14

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Attachment: Talos comparison screenshot

The Talos data has some concerning regressions (see screenshot):

tp5o responsiveness opt e10s stylo: 17% (linux64), 12% (win10-64)
tabpaint opt e10s stylo: 5% (linux64)

Looking at a graph of recent results for the tp5o responsiveness test, that regression is definitely above the trend line. (I'll attach a second screenshot of the graph)

I think we should try reverting the change from Photon to WebExtension page action, and see if that removes the regressions.

Comment 15

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Attachment: Talos trend screenshot

Attached is the trend graph for linux64 tp5o responsiveness from the past 2 days. The trend line is fairly consistent; our results look to be a definite regression.

How to read this graph:

- highlighted purple dots: the 5 runs from this push (their average is 17%)
- other purple dots: other recent try Talos runs. ignore them
- orange dots: results from recent mozilla-central Talos runs

Comment 16

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

:chenba pointed out that we discussed these regressions in the pull request on Github: https://github.com/mozilla-services/screenshots/pull/3967

In that discussion, :kmag was not too concerned about the responsiveness regressions: https://github.com/mozilla-services/screenshots/pull/3967#issuecomment-379069692

Comment 17

[Barry Chen]

Comment on attachment 8982025 [details]
Bug 1465581 - Export Screenshots 33.0.0 to Firefox (translations only);

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

Comment 18

[Barry Chen]

Comment on attachment 8982026 [details]
Bug 1465581 - Export Screenshots 33.0.0 to Firefox (upgrade Raven to 3.25.2);

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

Comment 19

[Barry Chen]

Comment on attachment 8982027 [details]
Bug 1465581 - Export Screenshots 33.0.0 to Firefox (code excluding translations and Raven update);

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

Comment 20

[Barry Chen]

The switch to WebExtension page action has been removed from this export.  New build:

 - https://treeherder.mozilla.org/#/jobs?repo=try&revision=f52914af3cb5844d1ec7110fbe36ba299a3f4322
 - https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=f52914af3cb5844d1ec7110fbe36ba299a3f4322

Comment 21

[Barry Chen]

Comment on attachment 8982025 [details]
Bug 1465581 - Export Screenshots 33.0.0 to Firefox (translations only);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/248076/diff/3-4/

Comment 22

[Barry Chen]

Comment on attachment 8982026 [details]
Bug 1465581 - Export Screenshots 33.0.0 to Firefox (upgrade Raven to 3.25.2);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/248078/diff/3-4/

Comment 23

[Barry Chen]

Comment on attachment 8982027 [details]
Bug 1465581 - Export Screenshots 33.0.0 to Firefox (code excluding translations and Raven update);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/248080/diff/3-4/

Comment 24

[Barry Chen]

QA found a small regression (https://github.com/mozilla-services/screenshots/issues/4523) in the previous export.  This latest export includes a fix for that.

Try:
 - https://treeherder.mozilla.org/#/jobs?repo=try&revision=90806f37f1c6545242a4a742b7a4091d3cc81ad7
 - https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=90806f37f1c6545242a4a742b7a4091d3cc81ad7

Comment 25

[Barry Chen]

Comment on attachment 8982025 [details]
Bug 1465581 - Export Screenshots 33.0.0 to Firefox (translations only);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/248076/diff/4-5/

Comment 26

[Barry Chen]

Comment on attachment 8982026 [details]
Bug 1465581 - Export Screenshots 33.0.0 to Firefox (upgrade Raven to 3.25.2);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/248078/diff/4-5/

Comment 27

[Barry Chen]

Comment on attachment 8982027 [details]
Bug 1465581 - Export Screenshots 33.0.0 to Firefox (code excluding translations and Raven update);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/248080/diff/4-5/

Comment 28

[Barry Chen]

The latest export added a fix for https://github.com/mozilla-services/screenshots/issues/4549.

Try:
 - https://treeherder.mozilla.org/#/jobs?repo=try&revision=03cbcfeb34e011b69914a32f39181b27e3376e40
 - https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=03cbcfeb34e011b69914a32f39181b27e3376e40

Comment 29

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

The interdiffs look reasonable to me; merging.

Thanks again for taking on this chore, Barry 

Comment 30

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Comment on attachment 8982027 [details]
Bug 1465581 - Export Screenshots 33.0.0 to Firefox (code excluding translations and Raven update);

https://reviewboard.mozilla.org/r/248080/#review258602

Comment 31

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Comment on attachment 8982027 [details]
Bug 1465581 - Export Screenshots 33.0.0 to Firefox (code excluding translations and Raven update);

https://reviewboard.mozilla.org/r/248080/#review258604

Comment 32

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Comment on attachment 8982026 [details]
Bug 1465581 - Export Screenshots 33.0.0 to Firefox (upgrade Raven to 3.25.2);

https://reviewboard.mozilla.org/r/248078/#review258606

Looks like changing the MozReview-Commit-ID cleared the R+. Trying to re-R+ so I can autoland this.

Comment 33

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5d075fe82827
Export Screenshots 33.0.0 to Firefox (translations only); r=flod,ianbicking
https://hg.mozilla.org/integration/autoland/rev/3dda0c68e6bd
Export Screenshots 33.0.0 to Firefox (upgrade Raven to 3.25.2); r=ianbicking,_6a68
https://hg.mozilla.org/integration/autoland/rev/3f8b00d17eea
Export Screenshots 33.0.0 to Firefox (code excluding translations and Raven update); r=ianbicking,_6a68

Comment 34

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/5d075fe82827
https://hg.mozilla.org/mozilla-central/rev/3dda0c68e6bd
https://hg.mozilla.org/mozilla-central/rev/3f8b00d17eea