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.
Created attachment 373078 [details] [Image] Screenshot of Issue Red box is the not-so-good ordering :( Green box is a good ordering result :)
Created attachment 373079 [details] Patch
Created attachment 373083 [details] Patch
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?
Created attachment 374719 [details] Patch
Created attachment 374720 [details] [Image] After Patch is applied
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`).
Created attachment 374728 [details] Patch Simplify some styles
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
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
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
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
Created attachment 374745 [details] Patch Fix test :P
Comment on attachment 374745 [details] Patch Clearing flags on attachment: 374745 Committed r247790: <https://trac.webkit.org/changeset/247790>
All reviewed patches have been landed. Closing bug.
<rdar://problem/53513907>