WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
199308
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
Summary
Web Inspector: print the target of `console.screenshot` last so the target is...
Devin Rousso
Reported
2019-06-27 20:21:37 PDT
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.
Attachments
[Image] Screenshot of Issue
(906.78 KB, image/png)
2019-06-27 20:22 PDT
,
Devin Rousso
no flags
Details
Patch
(17.54 KB, patch)
2019-06-27 21:29 PDT
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
Patch
(18.02 KB, patch)
2019-06-27 21:50 PDT
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
Patch
(27.01 KB, patch)
2019-07-23 15:32 PDT
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
[Image] After Patch is applied
(1.06 MB, image/png)
2019-07-23 15:32 PDT
,
Devin Rousso
no flags
Details
Patch
(27.23 KB, patch)
2019-07-23 16:12 PDT
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews101 for mac-highsierra
(3.33 MB, application/zip)
2019-07-23 17:29 PDT
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews104 for mac-highsierra-wk2
(2.72 MB, application/zip)
2019-07-23 17:39 PDT
,
EWS Watchlist
no flags
Details
Patch
(28.01 KB, patch)
2019-07-23 17:47 PDT
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
Show Obsolete
(6)
View All
Add attachment
proposed patch, testcase, etc.
Devin Rousso
Comment 1
2019-06-27 20:22:37 PDT
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
Comment 2
2019-06-27 21:29:55 PDT
Created
attachment 373079
[details]
Patch
Devin Rousso
Comment 3
2019-06-27 21:50:54 PDT
Created
attachment 373083
[details]
Patch
Joseph Pecoraro
Comment 4
2019-07-22 13:16:35 PDT
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
Comment 5
2019-07-23 15:32:36 PDT
Created
attachment 374719
[details]
Patch
Devin Rousso
Comment 6
2019-07-23 15:32:50 PDT
Created
attachment 374720
[details]
[Image] After Patch is applied
Devin Rousso
Comment 7
2019-07-23 15:49:57 PDT
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
Comment 8
2019-07-23 16:12:41 PDT
Created
attachment 374728
[details]
Patch Simplify some styles
EWS Watchlist
Comment 9
2019-07-23 17:29:28 PDT
Comment hidden (obsolete)
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
Comment 10
2019-07-23 17:29:30 PDT
Comment hidden (obsolete)
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 11
2019-07-23 17:39:53 PDT
Comment hidden (obsolete)
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
Comment 12
2019-07-23 17:39:55 PDT
Comment hidden (obsolete)
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
Comment 13
2019-07-23 17:47:32 PDT
Created
attachment 374745
[details]
Patch Fix test :P
WebKit Commit Bot
Comment 14
2019-07-24 14:39:38 PDT
Comment on
attachment 374745
[details]
Patch Clearing flags on attachment: 374745 Committed
r247790
: <
https://trac.webkit.org/changeset/247790
>
WebKit Commit Bot
Comment 15
2019-07-24 14:39:40 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 16
2019-07-24 14:40:55 PDT
<
rdar://problem/53513907
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug