Attachment 373079 Details for Bug 199308 – Patch

[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:

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>

Actions: View | Formatted Diff | Diff

Attachments on bug 199308: 373078 | 373079 | 373083 | 374719 | 374720 | 374728 | 374740 | 374742 | 374745