1474006 - screenshot command in browser console is broken

Status: RESOLVED FIXED

(DevTools :: Console, defect, P3)

Description

[Sören Hentzschel]

Attachment: screenshot

The ":screenshot" command in the browser console does not work. As you can see in the attached screenshot the tabs have no color and the content area is blank. The browser console logged an error:

"Saved to /Users/cadeyrn/Downloads/Screen Shot 2018-07-06 at 22.35.37.png"
[Show/hide message details.] [Exception... "Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIAnnotationService.setPageAnnotation]"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: resource://gre/modules/DownloadHistory.jsm :: updateMetaData :: line 130"  data: no]

Comment 1

[Sebastian Zartner [:sebo]]

Attachment: Screenshot taken on Windows 10

FWIW, using Firefox 63.0a1 (2018-07-06) on Windows 10 there is no error logged and the browser tabs do have the correct background color.

Only the content area is blank, which is obviously an issue between chrome and content processes being split as noted at https://groups.google.com/d/msg/mozilla.dev.developer-tools/z2f_URkQxL0/aYZX4VK-AgAJ.

Sebastian

Comment 2

[Alexandre Poirot [:ochameau]]

While I totally understand that this is broken, I'm curious about why are you trying to use :screenshot from the browser console. If you want to take a screenshot of the page, you can do it from the regular web console.

Is it important for you to see both the page content *and* firefox UI around it?
Are you doing blogpost/communication/docs about Firefox UI/features?

Comment 3

[Sören Hentzschel]

> While I totally understand that this is broken, I'm curious about why are you trying to use :screenshot from the browser console.

To be honest only for QA reasons and because I didn't know that it's a command for the web console and not for the browser console. :D

> Is it important for you to see both the page content *and* firefox UI around it?

I would not say important but a capturing of content and Firefox UI would be a really nice thing. Since the integration of Firefox Screenshots I don't need a console command at all.

> Are you doing blogpost/communication/docs about Firefox UI/features?

Yes! I write a lot of blog articles about all Mozilla related topics, including articles about new Firefox features. ;)

Comment 4

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1474006 - Work in progress

This code shows a way to retrieve the content of the tab from the parent process by using the message manager.
It requires additional work to merge this screenshot into the parent process one by using Canvas API,
as well as polish and a test.

It mostly highlights how to do the cross process communication via message managers in order to retrieve the missing piece of the screenshot.

I'm not planning to finish this patch, so anyone is free to pick this up.

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

Comment 5

[Alexandre Poirot [:ochameau]]

Comment on attachment 8993606 [details]
Bug 1474006 - Work in progress

This code shows a way to retrieve the content of the tab from the parent process by using the message manager.
It requires additional work to merge this screenshot into the parent process one by using Canvas API,
as well as polish and a test.
As-is, this patch will replace :screenshot done from the browser console with a screenshot of the tab, but this time the screenshot is retrieve in the parent process, where the console actor runs for the browser console.

It mostly highlights how to do the cross process communication via message managers in order to retrieve the missing piece of the screenshot.

I'm not planning to finish this patch, so anyone is free to pick this up.

Comment 6

[Code Review Bot [:reviewbot]]

Comment on attachment 8993606 [details]
Bug 1474006 - Work in progress

https://reviewboard.mozilla.org/r/258304/#review265350


Code analysis found 12 defects in this patch:
 - 12 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: devtools/server/actors/webconsole/screenshot.js:52
(Diff revision 1)
>  /**
>   * This does the dirty work of creating a base64 string out of an
>   * area of the browser window
>   */
>  function createScreenshotData(document, args) {
> +  if (document.location.href.includes("browser.xul")) { // if we are in browser console codepath

Error: Line 52 exceeds the maximum line length of 90. [eslint: max-len]

::: devtools/server/actors/webconsole/screenshot.js:55
(Diff revision 1)
>   */
>  function createScreenshotData(document, args) {
> +  if (document.location.href.includes("browser.xul")) { // if we are in browser console codepath
> +    // Get the message manager for the current tab
> +    // document should be a reference to the topmost firefox window (I think)
> +    // otherwise `Services.wm.getMostRecentWindow("navigator:browser")`  returns it for sure

Error: Line 55 exceeds the maximum line length of 90. [eslint: max-len]

::: devtools/server/actors/webconsole/screenshot.js:57
(Diff revision 1)
> +  if (document.location.href.includes("browser.xul")) { // if we are in browser console codepath
> +    // Get the message manager for the current tab
> +    // document should be a reference to the topmost firefox window (I think)
> +    // otherwise `Services.wm.getMostRecentWindow("navigator:browser")`  returns it for sure
> +    const { messageManager } = document.defaultView.gBrowser.selectedBrowser;
> +    // you may already be used to such data + stringified function trick, we use that in ContentTask

Error: Line 57 exceeds the maximum line length of 90. [eslint: max-len]

::: devtools/server/actors/webconsole/screenshot.js:58
(Diff revision 1)
> +    // Get the message manager for the current tab
> +    // document should be a reference to the topmost firefox window (I think)
> +    // otherwise `Services.wm.getMostRecentWindow("navigator:browser")`  returns it for sure
> +    const { messageManager } = document.defaultView.gBrowser.selectedBrowser;
> +    // you may already be used to such data + stringified function trick, we use that in ContentTask
> +    // an important thing to know is that stringification doesn't like comments because it stringify without "\n"

Error: Line 58 exceeds the maximum line length of 90. [eslint: max-len]

::: devtools/server/actors/webconsole/screenshot.js:59
(Diff revision 1)
> +    // document should be a reference to the topmost firefox window (I think)
> +    // otherwise `Services.wm.getMostRecentWindow("navigator:browser")`  returns it for sure
> +    const { messageManager } = document.defaultView.gBrowser.selectedBrowser;
> +    // you may already be used to such data + stringified function trick, we use that in ContentTask
> +    // an important thing to know is that stringification doesn't like comments because it stringify without "\n"
> +    // It is very hard to have comments in them, /* */ are supposed to work but I wasn't able to have them working here...

Error: Line 59 exceeds the maximum line length of 90. [eslint: max-len]

::: devtools/server/actors/webconsole/screenshot.js:63
(Diff revision 1)
> +    // an important thing to know is that stringification doesn't like comments because it stringify without "\n"
> +    // It is very hard to have comments in them, /* */ are supposed to work but I wasn't able to have them working here...
> +    /* In a "frame script", you have access to some special globals like `content` */
> +    /*   which is the window object of the current content document */
> +    /*   More globals listed here: https://searchfox.org/mozilla-central/source/dom/chrome-webidl/MessageManager.webidl#464-492 */
> +    messageManager.loadFrameScript("data:,new " + function () {

Error: Unexpected space before function parentheses. [eslint: space-before-function-paren]

::: devtools/server/actors/webconsole/screenshot.js:65
(Diff revision 1)
> +    /* In a "frame script", you have access to some special globals like `content` */
> +    /*   which is the window object of the current content document */
> +    /*   More globals listed here: https://searchfox.org/mozilla-central/source/dom/chrome-webidl/MessageManager.webidl#464-492 */
> +    messageManager.loadFrameScript("data:,new " + function () {
> +      const { require } = ChromeUtils.import("resource://devtools/shared/Loader.jsm", {});
> +      const { createScreenshotData } = require("devtools/server/actors/webconsole/screenshot");

Error: 'createScreenshotData' is already declared in the upper scope. [eslint: no-shadow]

::: devtools/server/actors/webconsole/screenshot.js:67
(Diff revision 1)
> +    /*   More globals listed here: https://searchfox.org/mozilla-central/source/dom/chrome-webidl/MessageManager.webidl#464-492 */
> +    messageManager.loadFrameScript("data:,new " + function () {
> +      const { require } = ChromeUtils.import("resource://devtools/shared/Loader.jsm", {});
> +      const { createScreenshotData } = require("devtools/server/actors/webconsole/screenshot");
> +      const onMessage = async (msg) => {
> +        removeMessageListener("debug:screenshot-command", onMessage);

Error: 'removeMessageListener' is not defined. [eslint: no-undef]

::: devtools/server/actors/webconsole/screenshot.js:69
(Diff revision 1)
> +      const { require } = ChromeUtils.import("resource://devtools/shared/Loader.jsm", {});
> +      const { createScreenshotData } = require("devtools/server/actors/webconsole/screenshot");
> +      const onMessage = async (msg) => {
> +        removeMessageListener("debug:screenshot-command", onMessage);
> +
> +        const data = await createScreenshotData(content.document, msg.data);

Error: 'content' is not defined. [eslint: no-undef]

::: devtools/server/actors/webconsole/screenshot.js:70
(Diff revision 1)
> +      const { createScreenshotData } = require("devtools/server/actors/webconsole/screenshot");
> +      const onMessage = async (msg) => {
> +        removeMessageListener("debug:screenshot-command", onMessage);
> +
> +        const data = await createScreenshotData(content.document, msg.data);
> +        sendAsyncMessage("debug:screenshot-command", data);

Error: 'sendAsyncMessage' is not defined. [eslint: no-undef]

::: devtools/server/actors/webconsole/screenshot.js:72
(Diff revision 1)
> +        removeMessageListener("debug:screenshot-command", onMessage);
> +
> +        const data = await createScreenshotData(content.document, msg.data);
> +        sendAsyncMessage("debug:screenshot-command", data);
> +      };
> +      addMessageListener("debug:screenshot-command", onMessage);

Error: 'addMessageListener' is not defined. [eslint: no-undef]

::: devtools/server/actors/webconsole/screenshot.js:73
(Diff revision 1)
> +
> +        const data = await createScreenshotData(content.document, msg.data);
> +        sendAsyncMessage("debug:screenshot-command", data);
> +      };
> +      addMessageListener("debug:screenshot-command", onMessage);
> +    }, false /* false as we want to execute it only against the current document and not the next one to come */);

Error: Line 73 exceeds the maximum line length of 90. [eslint: max-len]

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Please note that there is a new API called drawSnapshot() which will make that work. In Marionette I call this API always from the parent process (bug 1559592), and it correctly renders the image with both the output from chrome and content included. Just note that this new API doesn't have tests yet, and as such still has some remaining issues. Therefore see the dependency list on the before-mentioned bug.

Comment 8

[Nicolas Chevobbe [:nchevobbe]]

Bug 1678483 will probably fix this, and we could repurpose this bug to add a test of the :screenshot command in the browser console (which could look very similar to what we'll do in Bug 1686720)

Comment 9

[Chris Peterson [:cpeterson]]

Tracking dt-fission-m3-reserve bugs for Fission M8 (blocking Release channel experiment, but not Beta experiment).

Comment 10

[Chris Peterson [:cpeterson]]

Moving "dt-fission-m3-reserve" bugs to "dt-fission-future" because they don't block Fission MVP.

Comment 11

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1474006 - [devtools] Enable screenshots for Browser Console and Browser Toolbox. r=jdescottes.

In order to enable screenshots from the browser console and browser toolbox, we
make a small change in the simulateCameraFlash, as the browsing context we
pass to it does not have a topFrameElement property (so we use the top chrome window
document element instead).

We also have to register browser actors when starting the browser console so it
can have access to the screenshot actor.

A simple test is added for :screenshot in the Browser Console.

Depends on D103613

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ed5759f4e21f
[devtools] Enable screenshots for Browser Console and Browser Toolbox. r=jdescottes.

Comment 13

[Alexandru Michis [:malexandru]]

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