1689951 - Display a notification of screenshot warning and error messages

Status: RESOLVED FIXED

(DevTools :: General, task, P3)

Description

[Nicolas Chevobbe [:nchevobbe]]

A few things can go wrong when taking a screenshot, and when that's the case, we usually print a message in the console but the user may not have it visible (screenshot can be taken from any panel with the screenshot button in the toolbox, from the inspector, and from RDM).
It would be nice to show those message in another area, when the user isn't using the console.
Looks like the notification box could be the right place to do that (and would work with RDM + closed toolbox too)

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Attachment: mockup

here's what it could look like

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

In Bug 1678483, we are putting in place a mechanism to retrieve all the messages emitted during the screenshot process, so we could consume that to show the notifications.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I like the idea.

Nicola, perhaps this could be a good first bug? (with proper instructions provided)

Honza

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

mh, not sure. There are multiple callsites to take care of and tests to implement, and it depends on another bug at the moment

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1689951 - [devtools] Display notification for screenshot warnings and error messages. r=jdescottes.

This patch make the warning and error messages visible when such messages were
emitted, for the RDM screenshot button, the toolbar screenshot button and the
inspector screenshot node context menu entry.
In the case of the toolbar screenshot button in the browser toolbox, we also
display the saved messages so users have a way to know where the file was saved.
We don't do anything more for :screenshot command in the console, as the messages
are already displayed directly in the console output.
Tests are added for the different use case. The RDM one is a bit different as
we can't have a viewport taller than 9999px, so we bump the dpr to make capture-screenshot
downsize it (as such image would be too big).

We take this opportunity to fix the takeNodeScreenshot helper, which was adding
a "load" event listener after setting the src on an image, which can lead to races.

Depends on D104004

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/09b2d1c8bba2
[devtools] Display notification for screenshot warnings and error messages. r=jdescottes.

Comment 8

[Sandor Molnar[:smolnar]]

Backed out for causing dt failures in browser_screenshot_button_warning

Backout link: https://hg.mozilla.org/integration/autoland/rev/569826c0fd47a0e196057500c0873fab31253b70

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=bxyV3Fs5QzqPQAxGMW6FIQ.0&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&revision=09b2d1c8bba2075330790cf05b641fce84712870

Failure log: https://treeherder.mozilla.org/logviewer?job_id=329497600&repo=autoland&lineNumber=11224

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/933da04d9b3f
[devtools] Display notification for screenshot warnings and error messages. r=jdescottes.

Comment 10

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/933da04d9b3f