WebKit Bugzilla
Attachment 373079 Details for
Bug 199308
: Web Inspector: print the target of `console.screenshot` last so the target is the closest item to the image
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-199308-20190627212955.patch (text/plain), 17.54 KB, created by
Devin Rousso
on 2019-06-27 21:29:55 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Devin Rousso
Created:
2019-06-27 21:29:55 PDT
Size:
17.54 KB
patch
obsolete
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index fae0ed5366f33554d8b03693244b369735a3f8c9..943e9d02d3baf234de80cc47dd941d20d83529b2 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,28 @@ >+2019-06-27 Devin Rousso <drousso@apple.com> >+ >+ Web Inspector: print the target of `console.screenshot` last so the target is the closest item to the image >+ https://bugs.webkit.org/show_bug.cgi?id=199308 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Right now, evaluating `console.screenshot(document, "test", 1);` will log a message to the >+ console with `#document`, `"test"`, and `1`, all on different lines (togglable by a >+ disclosure triangle) since `#document` isn't stringifiable. >+ >+ The ideal situation would be to have `"test"` and `1` logged on the same line, and then have >+ `#document` be in a disclosure triangle. This way, you can "label" your images using >+ additional arguments (e.g. `console.screenshot(document.images[1], "second image");`), as >+ well as provide other data. >+ >+ If the only argument was the target, it should print as if it was `console.log(target);`. >+ >+ If there are no arguments, it should print the text "Viewport"` before the image. >+ >+ Test: inspector/console/console-screenshot.html >+ >+ * page/PageConsoleClient.cpp: >+ (WebCore::PageConsoleClient::screenshot): >+ > 2019-06-27 Fujii Hironori <Hironori.Fujii@sony.com> > > [WinCairo][MediaFoundation] Stop using soft linking for Media Foundation >diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog >index 915cc2e7361997beadd4d59d0c8eb1df4acfce81..aa6acac72c08a5e2edeb97c2f217b9514686ceb0 100644 >--- a/Source/WebInspectorUI/ChangeLog >+++ b/Source/WebInspectorUI/ChangeLog >@@ -1,3 +1,16 @@ >+2019-06-27 Devin Rousso <drousso@apple.com> >+ >+ Web Inspector: print the target of `console.screenshot` last so the target is the closest item to the image >+ https://bugs.webkit.org/show_bug.cgi?id=199308 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * UserInterface/Views/ConsoleMessageView.js: >+ (WI.ConsoleMessageView.prototype._appendMessageTextAndArguments): >+ Allow `ConsoleMessage.MessageType.Image` to be an `ConsoleMessage.MessageLevel.Error`, and >+ print the message (and extra parameters) in that case. >+ Drive-by: reorganize the switch-case so all paths have the same `default` case. >+ > 2019-06-27 Beth Dakin <bdakin@apple.com> > > Upstream use of MACCATALYST >diff --git a/Source/WebCore/page/PageConsoleClient.cpp b/Source/WebCore/page/PageConsoleClient.cpp >index c3b1d3692086770fb1780c490e47748ebd91257a..29af979bf606965ac85a973a31441931a9a32cc3 100644 >--- a/Source/WebCore/page/PageConsoleClient.cpp >+++ b/Source/WebCore/page/PageConsoleClient.cpp >@@ -263,41 +263,53 @@ void PageConsoleClient::screenshot(JSC::ExecState* state, Ref<ScriptArguments>&& > { > FAST_RETURN_IF_NO_FRONTENDS(void()); > >- Frame& frame = m_page.mainFrame(); >- >+ bool cannotScreenshot = false; >+ String messageText; > std::unique_ptr<ImageBuffer> snapshot; > >- auto* target = objectArgumentAt(arguments, 0); >- if (target) { >- auto* node = JSNode::toWrapped(state->vm(), target); >- if (!node) >- return; >+ auto argumentCount = arguments->argumentCount(); >+ if (argumentCount) { >+ // Save screenshot target before reordering arguments. >+ auto target = arguments->argumentAt(0); >+ >+ if (argumentCount > 1) { >+ Vector<JSC::Strong<JSC::Unknown>> reordered; >+ for (size_t i = 1; i < argumentCount; ++i) >+ reordered.append({ state->vm(), arguments->argumentAt(i) }); >+ reordered.append({ state->vm(), target }); >+ arguments = ScriptArguments::create(*state, WTFMove(reordered)); >+ } >+ >+ // Get new first argument after reordering. >+ arguments->getFirstArgumentAsString(messageText); >+ >+ if (auto* node = JSNode::toWrapped(state->vm(), target)) { >+ snapshot = WebCore::snapshotNode(m_page.mainFrame(), *node); >+ cannotScreenshot = !snapshot; >+ } >+ } >+ >+ if (!cannotScreenshot && !snapshot) { >+ if (messageText.isEmpty()) >+ messageText = "Viewport"_s; > >- snapshot = WebCore::snapshotNode(frame, *node); >- } else { > // If no target is provided, capture an image of the viewport. >- IntRect imageRect(IntPoint::zero(), frame.view()->sizeForVisibleContent()); >- snapshot = WebCore::snapshotFrameRect(frame, imageRect, SnapshotOptionsInViewCoordinates); >+ IntRect imageRect(IntPoint::zero(), m_page.mainFrame().view()->sizeForVisibleContent()); >+ snapshot = WebCore::snapshotFrameRect(m_page.mainFrame(), imageRect, SnapshotOptionsInViewCoordinates); > } > > if (!snapshot) { >- addMessage(std::make_unique<Inspector::ConsoleMessage>(MessageSource::ConsoleAPI, MessageType::Log, MessageLevel::Error, "Could not capture screenshot"_s, arguments.copyRef())); >+ addMessage(std::make_unique<Inspector::ConsoleMessage>(MessageSource::ConsoleAPI, MessageType::Image, MessageLevel::Error, "Could not capture screenshot"_s, WTFMove(arguments))); > return; > } > > String dataURL = snapshot->toDataURL("image/png"_s, WTF::nullopt, PreserveResolution::Yes); > if (dataURL.isEmpty()) { >- addMessage(std::make_unique<Inspector::ConsoleMessage>(MessageSource::ConsoleAPI, MessageType::Log, MessageLevel::Error, "Could not capture screenshot"_s, arguments.copyRef())); >+ addMessage(std::make_unique<Inspector::ConsoleMessage>(MessageSource::ConsoleAPI, MessageType::Image, MessageLevel::Error, "Could not capture screenshot"_s, WTFMove(arguments))); > return; > } > >- if (target) { >- // Log the argument before sending the image for it. >- String messageText; >- arguments->getFirstArgumentAsString(messageText); >- addMessage(std::make_unique<Inspector::ConsoleMessage>(MessageSource::ConsoleAPI, MessageType::Log, MessageLevel::Log, messageText, arguments.copyRef())); >- } >- >+ addMessage(std::make_unique<Inspector::ConsoleMessage>(MessageSource::ConsoleAPI, MessageType::Log, MessageLevel::Log, messageText, WTFMove(arguments))); > addMessage(std::make_unique<Inspector::ConsoleMessage>(MessageSource::ConsoleAPI, MessageType::Image, MessageLevel::Log, dataURL)); > } > >diff --git a/Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js b/Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js >index 6f0a7dd3f452ef0fc20972353db4ced48077856a..33b3c1634ec7709aea29a9bc6ac7906b1fd037e7 100644 >--- a/Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js >+++ b/Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js >@@ -256,7 +256,7 @@ WI.ConsoleMessageView = class ConsoleMessageView extends WI.Object > args = args.concat(this._message.parameters); > } > this._appendFormattedArguments(element, args); >- break; >+ return; > > case WI.ConsoleMessage.MessageType.Assert: > var args = [WI.UIString("Assertion Failed")]; >@@ -268,54 +268,58 @@ WI.ConsoleMessageView = class ConsoleMessageView extends WI.Object > args = args.concat(this._message.parameters); > } > this._appendFormattedArguments(element, args); >- break; >+ return; > > case WI.ConsoleMessage.MessageType.Dir: > var obj = this._message.parameters ? this._message.parameters[0] : undefined; > this._appendFormattedArguments(element, ["%O", obj]); >- break; >+ return; > > case WI.ConsoleMessage.MessageType.Table: > var args = this._message.parameters; > element.appendChild(this._formatParameterAsTable(args)); > this._extraParameters = null; >- break; >+ return; > > case WI.ConsoleMessage.MessageType.StartGroup: > case WI.ConsoleMessage.MessageType.StartGroupCollapsed: > var args = this._message.parameters || [this._message.messageText || WI.UIString("Group")]; > this._formatWithSubstitutionString(args, element); > this._extraParameters = null; >- break; >+ return; > > case WI.ConsoleMessage.MessageType.Timing: { > let args = [this._message.messageText]; > if (this._extraParameters) > args = args.concat(this._extraParameters); > this._appendFormattedArguments(element, args); >- break; >+ return; > } > > case WI.ConsoleMessage.MessageType.Image: { >- let img = element.appendChild(document.createElement("img")); >- img.classList.add("console-image", "show-grid"); >- img.src = this._message.messageText; >- img.setAttribute("filename", WI.FileUtilities.screenshotString() + ".png"); >- img.addEventListener("load", (event) => { >- if (img.width >= img.height) >- img.width = img.width / window.devicePixelRatio; >- else >- img.height = img.height / window.devicePixelRatio; >- }); >- break; >- } >+ if (this._message.level === WI.ConsoleMessage.MessageLevel.Log) { >+ let img = element.appendChild(document.createElement("img")); >+ img.classList.add("console-image", "show-grid"); >+ img.src = this._message.messageText; >+ img.setAttribute("filename", WI.FileUtilities.screenshotString() + ".png"); >+ img.addEventListener("load", (event) => { >+ if (img.width >= img.height) >+ img.width = img.width / window.devicePixelRatio; >+ else >+ img.height = img.height / window.devicePixelRatio; >+ }); >+ return; >+ } > >- default: >- var args = this._message.parameters || [this._message.messageText]; >- this._appendFormattedArguments(element, args); >- break; >+ if (this._message.level === WI.ConsoleMessage.MessageLevel.Error) { >+ let args = [this._message.messageText]; >+ if (this._extraParameters) >+ args = args.concat(this._extraParameters); >+ this._appendFormattedArguments(element, args); >+ return; >+ } >+ } > } >- return; > } > > // FIXME: Better handle WI.ConsoleMessage.MessageSource.Network once it has request info. >diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog >index 039dd1f6a0151c9a91959468da1349f4e53d1109..5c349573884b36d5623be25c01fcf6b0089b763d 100644 >--- a/LayoutTests/ChangeLog >+++ b/LayoutTests/ChangeLog >@@ -1,3 +1,13 @@ >+2019-06-27 Devin Rousso <drousso@apple.com> >+ >+ Web Inspector: print the target of `console.screenshot` last so the target is the closest item to the image >+ https://bugs.webkit.org/show_bug.cgi?id=199308 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * inspector/console/console-screenshot.html: >+ * inspector/console/console-screenshot-expected.txt: >+ > 2019-06-27 Ryosuke Niwa <rniwa@webkit.org> > > Fix the test added in r246868 on iOS debug simulator. >diff --git a/LayoutTests/inspector/console/console-screenshot-expected.txt b/LayoutTests/inspector/console/console-screenshot-expected.txt >index 0e9e3805c5e20953fbbfd8eaff31f0b3915c3e6f..3801375e3c0023c3ef500e4f6216153f25e1f229 100644 >--- a/LayoutTests/inspector/console/console-screenshot-expected.txt >+++ b/LayoutTests/inspector/console/console-screenshot-expected.txt >@@ -1,21 +1,32 @@ > CONSOLE MESSAGE: [object HTMLDivElement] >-CONSOLE MESSAGE: [object HTMLDivElement] >-CONSOLE MESSAGE: Could not capture screenshot >+CONSOLE MESSAGE: test >+CONSOLE MESSAGE: Viewport >+CONSOLE MESSAGE: 42 > Tests for the console.screenshot API. > > > == Running test suite: console.screenshot >--- Running test case: console.screenshot.SingleNode >+-- Running test case: console.screenshot.Node.SingleArgument > PASS: The added message should be an image. > PASS: The image should not be empty. >+PASS: The image width should be 2px. >+PASS: The image height should be 2px. > >--- Running test case: console.screenshot.MultipleNodes >+-- Running test case: console.screenshot.Node.MultipleArguments > PASS: The added message should be an image. > PASS: The image should not be empty. >+PASS: The image width should be 2px. >+PASS: The image height should be 2px. > >--- Running test case: console.screenshot.DetachedNode >+-- Running test case: console.screenshot.Node.DetachedNonScreenshotable > PASS: Could not capture screenshot > >+-- Running test case: console.screenshot.NonScreenshotableTarget >+PASS: The added message should be an image. >+PASS: The image should not be empty. >+PASS: The image width should be greater than 2px. >+PASS: The image height should be greater than 2px. >+ > -- Running test case: console.screenshot.NoArguments > PASS: The added message should be an image. > PASS: The image should not be empty. >diff --git a/LayoutTests/inspector/console/console-screenshot.html b/LayoutTests/inspector/console/console-screenshot.html >index c3e9ef3ecb3bef10f61986de8be1209bbca6c574..4e89533283e601a1a55a891c0316cbe8d0dea9c5 100644 >--- a/LayoutTests/inspector/console/console-screenshot.html >+++ b/LayoutTests/inspector/console/console-screenshot.html >@@ -4,10 +4,10 @@ > <script src="../../http/tests/inspector/resources/inspector-test.js"></script> > <script> > >-function createDetachedTest3() >+function createDetachedTest() > { > let div = document.createElement("div"); >- div.id = "test3"; >+ div.id = "detached"; > return div; > } > >@@ -48,37 +48,49 @@ function test() > } > > addTest({ >- name: "console.screenshot.SingleNode", >- expression: `console.screenshot(document.querySelector("#test1"))`, >+ name: "console.screenshot.Node.SingleArgument", >+ expression: `console.screenshot(document.querySelector("#testNode"))`, > async imageMessageAddedCallback(message) { > InspectorTest.expectNotEqual(message.messageText, "data:", "The image should not be empty."); > > let img = await WI.ImageUtilities.promisifyLoad(message.messageText); >- InspectorTest.assert(img.width === 2, "The image width should be 2px."); >- InspectorTest.assert(img.height === 2, "The image height should be 2px."); >+ InspectorTest.expectEqual(img.width, 2, "The image width should be 2px."); >+ InspectorTest.expectEqual(img.height, 2, "The image height should be 2px."); > }, > }); > > addTest({ >- name: "console.screenshot.MultipleNodes", >- expression: `console.screenshot(document.querySelector("#test2"), document.querySelector("#test1"))`, >+ name: "console.screenshot.Node.MultipleArguments", >+ expression: `console.screenshot(document.querySelector("#testNode"), "test")`, > async imageMessageAddedCallback(message) { > InspectorTest.expectNotEqual(message.messageText, "data:", "The image should not be empty."); > > let img = await WI.ImageUtilities.promisifyLoad(message.messageText); >- InspectorTest.assert(img.width === 2, "The image width should be 2px."); >- InspectorTest.assert(img.height === 2, "The image height should be 2px."); >+ InspectorTest.expectEqual(img.width, 2, "The image width should be 2px."); >+ InspectorTest.expectEqual(img.height, 2, "The image height should be 2px."); > }, > }); > > addTest({ >- name: "console.screenshot.DetachedNode", >- expression: `console.screenshot(createDetachedTest3())`, >+ name: "console.screenshot.Node.DetachedNonScreenshotable", >+ expression: `console.screenshot(createDetachedTest())`, > shouldError: true, > }); > > addTest({ >- name: "console.screenshot.NoArguments", >+ name: "console.screenshot.NonScreenshotableTarget", >+ expression: `console.screenshot(42)`, >+ async imageMessageAddedCallback(message) { >+ InspectorTest.expectNotEqual(message.messageText, "data:", "The image should not be empty."); >+ >+ let img = await WI.ImageUtilities.promisifyLoad(message.messageText); >+ InspectorTest.expectGreaterThan(img.width, 2, "The image width should be greater than 2px."); >+ InspectorTest.expectGreaterThan(img.height, 2, "The image height should be greater than 2px."); >+ }, >+ }); >+ >+ addTest({ >+ name: "console.screenshot.NoArguments", > expression: `console.screenshot()`, > async imageMessageAddedCallback(message) { > InspectorTest.expectNotEqual(message.messageText, "data:", "The image should not be empty."); >@@ -95,25 +107,18 @@ function test() > </head> > <body onload="runTest()"> > <p>Tests for the console.screenshot API.</p> >- <div id="test1"></div> >- <div id="test2"></div> >- <div id="test3"></div> >+ <div id="testNode"></div> > <style> >- #test1 { >+ #testNode { > width: 2px; > height: 2px; > background-color: red; > } >- #test2 { >+ #detached { > width: 2px; > height: 2px; > background-color: blue; > } >- #test3 { >- width: 2px; >- height: 2px; >- background-color: green; >- } > </style> > </body> > </html>
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 199308
:
373078
|
373079
|
373083
|
374719
|
374720
|
374728
|
374740
|
374742
|
374745