Bug 199308 – Web Inspector: print the target of `console.screenshot` last so the target is the closest item to the image

• Component: Web Inspector
• Status: RESOLVED FIXED

[Devin Rousso]

Right now, evaluating

```
   console.screenshot(document, "test", 1);
```

will log a message to the console with `#document`, `"test"`, and `1`, all of which are printed on different lines, togglable by a disclosure triangle.

The ideal situation would be to have `"test"` and `1` logged on the same line, and then have `#document` in the disclosure after that.  This way, you can "label" your images (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);` (no disclosure triangle).

If there are no arguments to `console.screenshot`, it should print the text "Viewport"` before the image.

[Devin Rousso]

Created attachment 373078 [details]
[Image] Screenshot of Issue

Red box is the not-so-good ordering :(

Green box is a good ordering result :)

[Devin Rousso]

Created attachment 373079 [details]
Patch

[Devin Rousso]

Created attachment 373083 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 373083 [details]
Patch

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

r=me, though this has changes to the default log path so ensure this doesn't break regular logging on some pages.

> Source/WebCore/page/PageConsoleClient.cpp:297
> +        if (messageText.isEmpty())
> +            messageText = "Viewport"_s;

We would probably want to make this a message type in the future so that the frontend can consider localizing this.

> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:325
>          // FIXME: Better handle WI.ConsoleMessage.MessageSource.Network once it has request info.

Should we make this an assert not reached?

[Devin Rousso]

Created attachment 374719 [details]
Patch

[Devin Rousso]

Created attachment 374720 [details]
[Image] After Patch is applied

[Devin Rousso]

Comment on attachment 373083 [details]
Patch

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

>> Source/WebCore/page/PageConsoleClient.cpp:297
>> +            messageText = "Viewport"_s;
> 
> We would probably want to make this a message type in the future so that the frontend can consider localizing this.

I changed the frontend to "hot-swap" this with a `WI.UIString("Viewport")`.

>> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:325
>>          // FIXME: Better handle WI.ConsoleMessage.MessageSource.Network once it has request info.
> 
> Should we make this an assert not reached?

I don't think we should, as this does get reached by other message types (`Log`, `DirXML`, `EndGroup`, `Profile`, `ProfileEnd`).

[Devin Rousso]

Created attachment 374728 [details]
Patch

Simplify some styles

[EWS Watchlist]

Comment on attachment 374728 [details]
Patch

Attachment 374728 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/12797687

New failing tests:
js/console.html

[EWS Watchlist]

Created attachment 374740 [details]
Archive of layout-test-results from ews101 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 374728 [details]
Patch

Attachment 374728 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/12797724

New failing tests:
js/console.html

[EWS Watchlist]

Created attachment 374742 [details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[Devin Rousso]

Created attachment 374745 [details]
Patch

Fix test :P

[WebKit Commit Bot]

Comment on attachment 374745 [details]
Patch

Clearing flags on attachment: 374745

Committed r247790: <https://trac.webkit.org/changeset/247790>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/53513907>