1867644 - Remember the method used for the last screenshot taken

Status: VERIFIED FIXED

(Firefox :: Screenshots, enhancement, P1)

Description

[Niklas Baumgardner [:niklas]]

We should remember the last screenshot taken and how it was saved.
For example, if it was a visible page screenshot and the image was copied, we should then focus the visible page button and the copy button on the next screenshot so the keyboard experienced can be streamlined for taking multiple screenshots in a row

Comment 1

[Niklas Baumgardner [:niklas]]

Attachment: Bug 1867644 - Focus last used button for screenshots UI. r=sfoster

Comment 2

[Sam Foster [:sfoster] (he/him)]

Just adjusting the bug title so we don't freak people out :)

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2b70f662a797
Focus last used button for screenshots UI. r=sfoster

Comment 5

[Iulian Moraru]

Backed out for causing bc failures on browser_overlay_keyboard_test.js.

• Push with failures

• Failure log

• Backout link

[task 2024-01-18T18:43:49.154Z] 18:43:49     INFO - TEST-PASS | browser/components/screenshots/tests/browser/browser_overlay_keyboard_test.js | Top dimension is correct - 10 == 10 - 
[task 2024-01-18T18:43:49.155Z] 18:43:49     INFO - Buffered messages finished
[task 2024-01-18T18:43:49.158Z] 18:43:49     INFO - TEST-UNEXPECTED-FAIL | browser/components/screenshots/tests/browser/browser_overlay_keyboard_test.js | Left dimension is correct - 25 == 20 - got 25, expected 20 (operator ==)
[task 2024-01-18T18:43:49.158Z] 18:43:49     INFO - Stack trace:
[task 2024-01-18T18:43:49.158Z] 18:43:49     INFO - is@resource://testing-common/SpecialPowersSandbox.sys.mjs:88:21
[task 2024-01-18T18:43:49.158Z] 18:43:49     INFO - assertSelectionRegionDimensions@chrome://mochitests/content/browser/browser/components/screenshots/tests/browser/browser_overlay_keyboard_test.js:182:15
[task 2024-01-18T18:43:49.158Z] 18:43:49     INFO - @chrome://mochitests/content/browser/browser/components/screenshots/tests/browser/browser_overlay_keyboard_test.js:219:44
[task 2024-01-18T18:43:49.158Z] 18:43:49     INFO - execute@resource://testing-common/SpecialPowersSandbox.sys.mjs:139:12
[task 2024-01-18T18:43:49.158Z] 18:43:49     INFO - _spawnTask@resource://testing-common/SpecialPowersChild.sys.mjs:1654:15
[task 2024-01-18T18:43:49.158Z] 18:43:49     INFO - receiveMessage@resource://testing-common/SpecialPowersChild.sys.mjs:257:21
[task 2024-01-18T18:43:49.159Z] 18:43:49     INFO - JSActor query*receiveMessage@resource://testing-common/SpecialPowersParent.sys.mjs:1385:14
[task 2024-01-18T18:43:49.159Z] 18:43:49     INFO - JSActor query*spawn@resource://testing-common/SpecialPowersChild.sys.mjs:1583:17
[task 2024-01-18T18:43:49.159Z] 18:43:49     INFO - test_moveRegionWithKeyboard/<@chrome://mochitests/content/browser/browser/components/screenshots/tests/browser/browser_overlay_keyboard_test.js:168:27
[task 2024-01-18T18:43:49.159Z] 18:43:49     INFO - async*withNewTab@resource://testing-common/BrowserTestUtils.sys.mjs:146:22
[task 2024-01-18T18:43:49.159Z] 18:43:49     INFO - async*test_moveRegionWithKeyboard@chrome://mochitests/content/browser/browser/components/screenshots/tests/browser/browser_overlay_keyboard_test.js:143:26
[task 2024-01-18T18:43:49.159Z] 18:43:49     INFO - handleTask@chrome://mochikit/content/browser-test.js:1138:26
[task 2024-01-18T18:43:49.159Z] 18:43:49     INFO - _runTaskBasedTest@chrome://mochikit/content/browser-test.js:1210:18
[task 2024-01-18T18:43:49.159Z] 18:43:49     INFO - async*Tester_execTest@chrome://mochikit/content/browser-test.js:1352:14
[task 2024-01-18T18:43:49.159Z] 18:43:49     INFO - nextTest/<@chrome://mochikit/content/browser-test.js:1127:14
[task 2024-01-18T18:43:49.159Z] 18:43:49     INFO - SimpleTest.waitForFocus/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:1058:13
[task 2024-01-18T18:43:49.160Z] 18:43:49     INFO - TEST-PASS | browser/components/screenshots/tests/browser/browser_overlay_keyboard_test.js | Bottom dimension is correct - 20 == 20 - 
[task 2024-01-18T18:43:49.161Z] 18:43:49     INFO - Not taking screenshot here: see the one that was previously logged
[task 2024-01-18T18:43:49.163Z] 18:43:49     INFO - TEST-UNEXPECTED-FAIL | browser/components/screenshots/tests/browser/browser_overlay_keyboard_test.js | Right dimension is correct - 25 == 30 - got 25, expected 30 (operator ==)
[task 2024-01-18T18:43:49.163Z] 18:43:49     INFO - Stack trace:
[task 2024-01-18T18:43:49.164Z] 18:43:49     INFO - is@resource://testing-common/SpecialPowersSandbox.sys.mjs:88:21
[task 2024-01-18T18:43:49.164Z] 18:43:49     INFO - assertSelectionRegionDimensions@chrome://mochitests/content/browser/browser/components/screenshots/tests/browser/browser_overlay_keyboard_test.js:192:15
[task 2024-01-18T18:43:49.164Z] 18:43:49     INFO - @chrome://mochitests/content/browser/browser/components/screenshots/tests/browser/browser_overlay_keyboard_test.js:219:44
[task 2024-01-18T18:43:49.164Z] 18:43:49     INFO - execute@resource://testing-common/SpecialPowersSandbox.sys.mjs:139:12
[task 2024-01-18T18:43:49.164Z] 18:43:49     INFO - _spawnTask@resource://testing-common/SpecialPowersChild.sys.mjs:1654:15
[task 2024-01-18T18:43:49.164Z] 18:43:49     INFO - receiveMessage@resource://testing-common/SpecialPowersChild.sys.mjs:257:21
[task 2024-01-18T18:43:49.164Z] 18:43:49     INFO - JSActor query*receiveMessage@resource://testing-common/SpecialPowersParent.sys.mjs:1385:14
[task 2024-01-18T18:43:49.164Z] 18:43:49     INFO - JSActor query*spawn@resource://testing-common/SpecialPowersChild.sys.mjs:1583:17
[task 2024-01-18T18:43:49.164Z] 18:43:49     INFO - test_moveRegionWithKeyboard/<@chrome://mochitests/content/browser/browser/components/screenshots/tests/browser/browser_overlay_keyboard_test.js:168:27
[task 2024-01-18T18:43:49.164Z] 18:43:49     INFO - async*withNewTab@resource://testing-common/BrowserTestUtils.sys.mjs:146:22
[task 2024-01-18T18:43:49.164Z] 18:43:49     INFO - async*test_moveRegionWithKeyboard@chrome://mochitests/content/browser/browser/components/screenshots/tests/browser/browser_overlay_keyboard_test.js:143:26
[task 2024-01-18T18:43:49.164Z] 18:43:49     INFO - handleTask@chrome://mochikit/content/browser-test.js:1138:26
[task 2024-01-18T18:43:49.164Z] 18:43:49     INFO - _runTaskBasedTest@chrome://mochikit/content/browser-test.js:1210:18
[task 2024-01-18T18:43:49.164Z] 18:43:49     INFO - async*Tester_execTest@chrome://mochikit/content/browser-test.js:1352:14
[task 2024-01-18T18:43:49.164Z] 18:43:49     INFO - nextTest/<@chrome://mochikit/content/browser-test.js:1127:14
[task 2024-01-18T18:43:49.164Z] 18:43:49     INFO - SimpleTest.waitForFocus/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:1058:13
[task 2024-01-18T18:43:49.165Z] 18:43:49     INFO - Not taking screenshot here: see the one that was previously logged
[task 2024-01-18T18:43:49.168Z] 18:43:49     INFO - TEST-UNEXPECTED-FAIL | browser/components/screenshots/tests/browser/browser_overlay_keyboard_test.js | Top dimension is correct - 25 == 20 - got 25, expected 20 (operator ==)
<...>

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c6a30d94edf2
Focus last used button for screenshots UI. r=sfoster

Comment 7

[Natalia Csoregi [:nataliaCs]]

Backed out for causing failures on browser_screenshots_drag_test.js

• backout: https://hg.mozilla.org/integration/autoland/rev/7fca0a309a3c24e0096edcefdd6fec267f6715ac
• push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=c6a30d94edf2df9a30cd7740d1720a73fc3f2b03&selectedTaskRun=S09jiIgCSD-spKmon-pLVA.0
• failure log: https://treeherder.mozilla.org/logviewer?job_id=443873515&repo=autoland&lineNumber=4968

[task 2024-01-19T01:34:28.815Z] 01:34:28     INFO - TEST-PASS | browser/components/screenshots/tests/browser/browser_screenshots_drag_test.js | Panel buttons are visible - 
[task 2024-01-19T01:34:28.816Z] 01:34:28     INFO - Buffered messages logged at 01:33:52
[task 2024-01-19T01:34:28.816Z] 01:34:28     INFO - Console message: [JavaScript Error: "TypeError: can't access property "getActor", browser.browsingContext.currentWindowGlobal is null" {file: "resource:///modules/ScreenshotsUtils.sys.mjs" line: 225}]
[task 2024-01-19T01:34:28.817Z] 01:34:28     INFO - getActor@resource:///modules/ScreenshotsUtils.sys.mjs:225:17
[task 2024-01-19T01:34:28.817Z] 01:34:28     INFO - closeOverlay@resource:///modules/ScreenshotsUtils.sys.mjs:534:22
[task 2024-01-19T01:34:28.817Z] 01:34:28     INFO - exit@resource:///modules/ScreenshotsUtils.sys.mjs:263:10
[task 2024-01-19T01:34:28.817Z] 01:34:28     INFO - didDestroy@resource:///modules/ScreenshotsUtils.sys.mjs:107:24
[task 2024-01-19T01:34:28.817Z] 01:34:28     INFO - 
[task 2024-01-19T01:34:28.817Z] 01:34:28     INFO - Buffered messages finished
[task 2024-01-19T01:34:28.818Z] 01:34:28     INFO - TEST-UNEXPECTED-FAIL | browser/components/screenshots/tests/browser/browser_screenshots_drag_test.js | Uncaught exception in test bound overlayCancelButton - undefined - threw exception: AbortError: Actor 'SpecialPowers' destroyed before query 'Spawn' was resolved
[task 2024-01-19T01:34:28.819Z] 01:34:28     INFO - Leaving test bound overlayCancelButton
[task 2024-01-19T01:34:28.819Z] 01:34:28     INFO - Entering test bound preserveBoxSizeWhenMovingOutOfWindowBounds
[task 2024-01-19T01:34:28.820Z] 01:34:28     INFO - GECKO(2158) | [Child 2696: Main Thread]: I/DocShellAndDOMWindowLeak ++DOCSHELL 7f6d8404f400 == 1 [pid = 2696] [id = 0]
[task 2024-01-19T01:34:28.821Z] 01:34:28     INFO - GECKO(2158) | [Child 2696: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 1 (7f6d84b7f020) [pid = 2696] [serial = 1] [outer = 0]
[task 2024-01-19T01:34:28.821Z] 01:34:28     INFO - GECKO(2158) | [Child 2696: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 2 (7f6d8404fc00) [pid = 2696] [serial = 2] [outer = 7f6d84b7f020]
[task 2024-01-19T01:34:28.822Z] 01:34:28     INFO - GECKO(2158) | [Child 2696: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 3 (7f6d84053c00) [pid = 2696] [serial = 3] [outer = 7f6d84b7f020]
[task 2024-01-19T01:34:28.822Z] 01:34:28     INFO - Console message: [JavaScript Warning: "This page is in Quirks Mode. Page layout may be impacted. For Standards Mode use “<!DOCTYPE html>”." {file: "https://example.com/browser/browser/components/screenshots/tests/browser/short-test-page.html" line: 0}]
[task 2024-01-19T01:34:28.824Z] 01:34:28     INFO - TEST-PASS | browser/components/screenshots/tests/browser/browser_screenshots_drag_test.js | Got dimensions back from the content - 

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9df2731a0530
Focus last used button for screenshots UI. r=sfoster

Comment 9

[Serban Stanca [:SerbanS]]

Backed out for causing mochitests failures in browser_contentAltClick.js on OSX.

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_contentAltClick.js | This test exceeded the timeout threshold. It should be rewritten or split up. If that's not possible, use requestLongerTimeout(N), but only as a last resort. -

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8bc6a4a3278a
Focus last used button for screenshots UI. r=sfoster

Comment 11

[Serban Stanca [:SerbanS]]

Relanded this as it turned out not be some serious failures from this changeset. Sorry for the inconveniences!

Comment 12

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/8bc6a4a3278a

Comment 13

[Pulsebot]

Backout by ffxbld-merge:
https://hg.mozilla.org/releases/mozilla-beta/rev/9c0c2aaab123
Backed out changeset 2b70f662a797 for causing bc failures on browser_overlay_keyboard_test.js. CLOSED TREE

Comment 14

[Simona Badau, Desktop QA]

While verifying this, I observed some behaviors that I'm not sure are intended or not:

1. Is it intentional for the "Copy" and "Download" options to always be remembered, regardless of whether the screenshot is captured using a click selection or by choosing the "Save visible" / "Save Full page" options"? (for e.g if I do a click selection and click on Copy, the Copy button will also be focused on the Save visible dialog).

2. Are the "Save visible" and "Save Full Screen" options also remembered? Currently, the focus consistently seems to be placed on the "Save Full Screen" option.

Niklas, could you please confirm whether the above behaviors are intended?

Comment 15

[Niklas Baumgardner [:niklas]]

(In reply to Simona Badau, Desktop QA from comment #14)

While verifying this, I observed some behaviors that I'm not sure are intended or not:

1. Is it intentional for the "Copy" and "Download" options to always be remembered, regardless of whether the screenshot is captured using a click selection or by choosing the "Save visible" / "Save Full page" options"? (for e.g if I do a click selection and click on Copy, the Copy button will also be focused on the Save visible dialog).

2. Are the "Save visible" and "Save Full Screen" options also remembered? Currently, the focus consistently seems to be placed on the "Save Full Screen" option.

Niklas, could you please confirm whether the above behaviors are intended?

1. That is correct. Copy/download are remembered regardless of the location (overlay/preview).
2. Yes, they should be remembered. I will file a bug for this

Comment 16

[Simona Badau, Desktop QA]

Based on comment 15, marking this as fixed on Firefox 123.0b4 - tested on macOS 13, Ubuntu 22.04 x64 and Windows 10 x64.