[webkit-reviews] review granted: [Bug 194279] Web Inspector: provide a way to capture a screenshot of a node from within the page : [Attachment 364646] Patch

[bugzilla-daemon at webkit.org]

Joseph Pecoraro <joepeck at webkit.org> has granted Devin Rousso
<drousso at apple.com>'s request for review:
Bug 194279: Web Inspector: provide a way to capture a screenshot of a node from
within the page
https://bugs.webkit.org/show_bug.cgi?id=194279

Attachment 364646: Patch

https://bugs.webkit.org/attachment.cgi?id=364646&action=review




--- Comment #41 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 364646
  --> https://bugs.webkit.org/attachment.cgi?id=364646
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=364646&action=review

r=me!

> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:292
> +		   let image =
element.appendChild(document.createElement("img"));

Style: I think we normally use `img` to match the element. Up to you.

> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:294
> +		   image.src = this._message.messageText;

Maybe we could use srcset to avoid having to do the logic in a "load" event?
But as is seems fine.

> LayoutTests/inspector/console/console-screenshot.html:30
> +	      
InspectorTest.evaluateInPage(`console.screenshot(document.querySelector("#test1
"))`)

We should have some more tests:

    • Node that is not attached to the DOM `let e =
document.createElement('a'); e.textContent = "test"; console.screenshot(e)`
    • Multiple nodes `console.screenshot(a, b)`
    • Screenshot with no arguments produces an expected width/height (I think
800x600 or something is the LayoutTest size