RESOLVED FIXED 122897
Web Inspector: logged objects are highlighted in blue
https://bugs.webkit.org/show_bug.cgi?id=122897
Summary Web Inspector: logged objects are highlighted in blue
Dave Coffin
Reported 2013-10-16 08:57:20 PDT
Created attachment 214365 [details] Screenshot of problem When highlighting objects in the console output, the entire object is highlighted blue.
Attachments
Screenshot of problem (17 bytes, text/plain)
2013-10-16 08:57 PDT, Dave Coffin
no flags
a suggestion for nicer looking and easier to read highlighting (264.46 KB, image/png)
2013-10-16 11:54 PDT, Dave Coffin
no flags
Patch (7.19 KB, patch)
2013-10-17 02:09 PDT, Antoine Quint
no flags
Screenshot showing result of patch (54.05 KB, image/png)
2013-10-17 02:13 PDT, Antoine Quint
no flags
Patch (7.16 KB, patch)
2013-10-17 13:49 PDT, Antoine Quint
no flags
Patch for landing (10.59 KB, patch)
2013-10-17 14:16 PDT, Antoine Quint
no flags
Radar WebKit Bug Importer
Comment 1 2013-10-16 08:58:23 PDT
Antoine Quint
Comment 2 2013-10-16 09:24:22 PDT
This is due to being able to select multiple rows in the console log to copy to the clipboard.
Timothy Hatcher
Comment 3 2013-10-16 11:32:00 PDT
We did attempt to fix it though, but it regressed. It should not select the row if you are clicking and arrow to expand the object.
Dave Coffin
Comment 4 2013-10-16 11:53:49 PDT
Tim- you did fix it I suppose, but the area you have to click is like 2px, and when you expand it doesn't unhighlight. I guess I have a larger problem with the highlighting in general, its ugly and you lose all syntax highlighting. I've attached a suggestion for better selecting in the console (IMO).
Dave Coffin
Comment 5 2013-10-16 11:54:19 PDT
Created attachment 214381 [details] a suggestion for nicer looking and easier to read highlighting
Joseph Pecoraro
Comment 6 2013-10-16 12:15:32 PDT
(In reply to comment #5) > Created an attachment (id=214381) [details] > a suggestion for nicer looking and easier to read highlighting I've consistently complained about this behavior in the JS Console. I like this highlighting idea!
Timothy Hatcher
Comment 7 2013-10-16 12:36:15 PDT
We can give it a try. I would try: background-color: highlight; that would keep it user controllable and use the system selection color.
Antoine Quint
Comment 8 2013-10-17 00:54:30 PDT
(In reply to comment #7) > We can give it a try. I would try: background-color: highlight; that would keep it user controllable and use the system selection color. I'm not sure about "background-color: highlight", seems to me we would probably run in the issue of contrast between the background color and the syntax-highlighted objects logged to the console.
Antoine Quint
Comment 9 2013-10-17 02:09:24 PDT
Antoine Quint
Comment 10 2013-10-17 02:10:35 PDT
Antoine Quint
Comment 11 2013-10-17 02:13:46 PDT
Created attachment 214438 [details] Screenshot showing result of patch
Timothy Hatcher
Comment 12 2013-10-17 08:57:25 PDT
(In reply to comment #8) > (In reply to comment #7) > > We can give it a try. I would try: background-color: highlight; that would keep it user controllable and use the system selection color. > > I'm not sure about "background-color: highlight", seems to me we would probably run in the issue of contrast between the background color and the syntax-highlighted objects logged to the console. It would be no different than a text selection (same color in fact).
Antoine Quint
Comment 13 2013-10-17 13:49:04 PDT
(In reply to comment #12) > (In reply to comment #8) > > (In reply to comment #7) > > > We can give it a try. I would try: background-color: highlight; that would keep it user controllable and use the system selection color. > > > > I'm not sure about "background-color: highlight", seems to me we would probably run in the issue of contrast between the background color and the syntax-highlighted objects logged to the console. > > It would be no different than a text selection (same color in fact). Updating the patch with "highlight".
Antoine Quint
Comment 14 2013-10-17 13:49:24 PDT
Joseph Pecoraro
Comment 15 2013-10-17 13:58:30 PDT
Comment on attachment 214501 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=214501&action=review r=me So we'll now get the light highlight everywhere? Does it look good everywhere? I think it'll be good, it'll especially be an improvement for DOMNode / Object interaction. > Source/WebInspectorUI/UserInterface/Images/UserInputPromptPreviousSelected.svg:-1 > -<?xml version="1.0" encoding="utf-8"?> Could you also remove these sag files from the Visual Studio project files? Lame, I know. But we don't need to be proactive adding files, just removing them =). shell> ack -a UserInputPromptPreviousSelected.svg WebInspectorUI.vcxproj/WebInspectorUI.vcxproj 539: <None Include="..\UserInterface\Images\UserInputPromptPreviousSelected.svg" /> WebInspectorUI.vcxproj/WebInspectorUI.vcxproj.filters 1698: <None Include="..\UserInterface\Images\UserInputPromptPreviousSelected.svg">
Antoine Quint
Comment 16 2013-10-17 14:04:08 PDT
(In reply to comment #15) > (From update of attachment 214501 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=214501&action=review > > r=me > > So we'll now get the light highlight everywhere? Does it look good everywhere? I think it'll be good, it'll especially be an improvement for DOMNode / Object interaction. > > > Source/WebInspectorUI/UserInterface/Images/UserInputPromptPreviousSelected.svg:-1 > > -<?xml version="1.0" encoding="utf-8"?> > > Could you also remove these sag files from the Visual Studio project files? Lame, I know. But we don't need to be proactive adding files, just removing them =). > > shell> ack -a UserInputPromptPreviousSelected.svg > WebInspectorUI.vcxproj/WebInspectorUI.vcxproj > 539: <None Include="..\UserInterface\Images\UserInputPromptPreviousSelected.svg" /> > > WebInspectorUI.vcxproj/WebInspectorUI.vcxproj.filters > 1698: <None Include="..\UserInterface\Images\UserInputPromptPreviousSelected.svg"> Will remove in commit.
Joseph Pecoraro
Comment 17 2013-10-17 14:04:13 PDT
Comment on attachment 214501 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=214501&action=review > Source/WebInspectorUI/UserInterface/LogContentView.css:-147 > -.console-messages:focus .console-group.collapsed .console-item.selected .console-group-title::before { > - background-image: -webkit-canvas(disclosure-triangle-small-closed-selected); > -} Ahh, optionally you're removing a lot of the only uses of -webkit-canvas foo-selected instances. You may be able to remove some of the specifications for CSS image generation from Main.js in WebInspector._generateDisclosureTriangleImages. WebInspector._generateDisclosureTriangleImages = function() { var specifications = {}; specifications["normal"] = {fillColor: [0, 0, 0, 0.5]}; specifications["normal-active"] = {fillColor: [0, 0, 0, 0.7]}; specifications["selected"] = {fillColor: [255, 255, 255, 0.8]}; specifications["selected-active"] = {fillColor: [255, 255, 255, 1]}; generateColoredImagesForCSS("Images/DisclosureTriangleSmallOpen.svg", specifications, 13, 13, "disclosure-triangle-small-open-"); generateColoredImagesForCSS("Images/DisclosureTriangleSmallClosed.svg", specifications, 13, 13, "disclosure-triangle-small-closed-"); generateColoredImagesForCSS("Images/DisclosureTriangleTinyOpen.svg", specifications, 8, 8, "disclosure-triangle-tiny-open-"); generateColoredImagesForCSS("Images/DisclosureTriangleTinyClosed.svg", specifications, 8, 8, "disclosure-triangle-tiny-closed-"); } It looks like you can entirely remove selected-active here. It is not used with disclosure-triangle's at all. And you can avoid "selected" for the top two, but probably still need it for the bottom two.
Antoine Quint
Comment 18 2013-10-17 14:16:36 PDT
Created attachment 214508 [details] Patch for landing
WebKit Commit Bot
Comment 19 2013-10-17 14:27:56 PDT
Comment on attachment 214508 [details] Patch for landing Clearing flags on attachment: 214508 Committed r157600: <http://trac.webkit.org/changeset/157600>
WebKit Commit Bot
Comment 20 2013-10-17 14:27:59 PDT
All reviewed patches have been landed. Closing bug.
Timothy Hatcher
Comment 21 2013-10-18 10:56:12 PDT
Comment on attachment 214501 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=214501&action=review >> Source/WebInspectorUI/UserInterface/LogContentView.css:-147 >> -} > > Ahh, optionally you're removing a lot of the only uses of -webkit-canvas foo-selected instances. You may be able to remove some of the specifications for CSS image generation from Main.js in WebInspector._generateDisclosureTriangleImages. > > WebInspector._generateDisclosureTriangleImages = function() > { > var specifications = {}; > specifications["normal"] = {fillColor: [0, 0, 0, 0.5]}; > specifications["normal-active"] = {fillColor: [0, 0, 0, 0.7]}; > specifications["selected"] = {fillColor: [255, 255, 255, 0.8]}; > specifications["selected-active"] = {fillColor: [255, 255, 255, 1]}; > > generateColoredImagesForCSS("Images/DisclosureTriangleSmallOpen.svg", specifications, 13, 13, "disclosure-triangle-small-open-"); > generateColoredImagesForCSS("Images/DisclosureTriangleSmallClosed.svg", specifications, 13, 13, "disclosure-triangle-small-closed-"); > > generateColoredImagesForCSS("Images/DisclosureTriangleTinyOpen.svg", specifications, 8, 8, "disclosure-triangle-tiny-open-"); > generateColoredImagesForCSS("Images/DisclosureTriangleTinyClosed.svg", specifications, 8, 8, "disclosure-triangle-tiny-closed-"); > } > > It looks like you can entirely remove selected-active here. It is not used with disclosure-triangle's at all. > And you can avoid "selected" for the top two, but probably still need it for the bottom two. Not completely. DOM tree and DataGrid use disclosure-triangle-tiny-open, which still has selected and active.
Joseph Pecoraro
Comment 22 2013-10-18 11:16:35 PDT
(In reply to comment #21) > (From update of attachment 214501 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=214501&action=review > > >> Source/WebInspectorUI/UserInterface/LogContentView.css:-147 > >> -} > > > > Ahh, optionally you're removing a lot of the only uses of -webkit-canvas foo-selected instances. You may be able to remove some of the specifications for CSS image generation from Main.js in WebInspector._generateDisclosureTriangleImages. > > > > WebInspector._generateDisclosureTriangleImages = function() > > { > > var specifications = {}; > > specifications["normal"] = {fillColor: [0, 0, 0, 0.5]}; > > specifications["normal-active"] = {fillColor: [0, 0, 0, 0.7]}; > > specifications["selected"] = {fillColor: [255, 255, 255, 0.8]}; > > specifications["selected-active"] = {fillColor: [255, 255, 255, 1]}; > > > > generateColoredImagesForCSS("Images/DisclosureTriangleSmallOpen.svg", specifications, 13, 13, "disclosure-triangle-small-open-"); > > generateColoredImagesForCSS("Images/DisclosureTriangleSmallClosed.svg", specifications, 13, 13, "disclosure-triangle-small-closed-"); > > > > generateColoredImagesForCSS("Images/DisclosureTriangleTinyOpen.svg", specifications, 8, 8, "disclosure-triangle-tiny-open-"); > > generateColoredImagesForCSS("Images/DisclosureTriangleTinyClosed.svg", specifications, 8, 8, "disclosure-triangle-tiny-closed-"); > > } > > > > It looks like you can entirely remove selected-active here. It is not used with disclosure-triangle's at all. > > And you can avoid "selected" for the top two, but probably still need it for the bottom two. > > Not completely. DOM tree and DataGrid use disclosure-triangle-tiny-open, which still has selected and active. I don't see anything using "selected-active". And yep, I was only saying "-small-" could avoid "selected", but "-tiny-" does still need it.
Note You need to log in before you can comment on or make changes to this bug.