Bug 107793 - Web Inspector: Add explanation for Console object expansion behaviour
Summary: Web Inspector: Add explanation for Console object expansion behaviour
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Sergey Ryazanov
URL: http://crbug.com/166134
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-23 23:47 PST by Sergey Ryazanov
Modified: 2013-01-25 04:37 PST (History)
10 users (show)

See Also:


Attachments
Patch (3.76 KB, patch)
2013-01-23 23:54 PST, Sergey Ryazanov
no flags Details | Formatted Diff | Diff
Screenshot of the feature (17.82 KB, image/png)
2013-01-23 23:55 PST, Sergey Ryazanov
no flags Details
Patch (3.75 KB, patch)
2013-01-24 01:53 PST, Sergey Ryazanov
no flags Details | Formatted Diff | Diff
Updated screenshot (17.59 KB, image/png)
2013-01-24 01:56 PST, Sergey Ryazanov
no flags Details
Patch (3.57 KB, patch)
2013-01-24 22:38 PST, Sergey Ryazanov
no flags Details | Formatted Diff | Diff
Updated screenshot (14.90 KB, image/png)
2013-01-24 22:39 PST, Sergey Ryazanov
no flags Details
Patch (3.64 KB, patch)
2013-01-25 03:55 PST, Sergey Ryazanov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sergey Ryazanov 2013-01-23 23:47:48 PST
http://crbug.com/166134

Steps to reproduce the problem:
1. Use console.log for logging an object.
2. Observe that the state of the object at the moment of logging is logged in italics.
3. Expand the object in the log with a mouse click. The state of the object is shown at the moment of expanding, not in italics.

Become confused if you do not know about rationale behind the behavior.

What is the expected behavior?
Explain the meaning of italics and why is the state of the object shown at the moment of expanding and not at the moment of logging.
Just a tiny informational icon with a hover popup would be enough.

What went wrong?
Developers without prior knowledge of the meaning of italics in console.log output, have a reason to believe that console.log shows the state at the time of logging and the expanding is only created to make the output manageble.

Did this work before? Yes Before  issue 50316  was fixed the situation was more confusing, but at least it was obviously confusing and wrong, so the developer would notice that. After  issue 50316  has been fixed, it is now subtly confusing.
Comment 1 Sergey Ryazanov 2013-01-23 23:54:51 PST
Created attachment 184419 [details]
Patch
Comment 2 Sergey Ryazanov 2013-01-23 23:55:52 PST
Created attachment 184420 [details]
Screenshot of the feature
Comment 3 Pavel Feldman 2013-01-24 01:14:22 PST
Comment on attachment 184419 [details]
Patch

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

> Source/WebCore/inspector/front-end/ConsoleMessage.js:308
> +        var note = section.element.querySelector(".header > .title").createChild("span", "object-info-state-note");

You should not reach out for ObjectPropertiesSection internals from here - it is better to have this code in the object properties section itself. You can add ObjectPropertiesSection.prototype.showExpansionHint(enabled) to control it from here, default it to false.

> Source/WebCore/inspector/front-end/inspector.css:1216
> +    display: none;

We try to be minimalistic wrt styles - sounds like alignment ones are not needed here.

> Source/WebCore/inspector/front-end/inspector.css:1224
> +    font-size: 10px;

We typically inherit font proeprties.
Comment 4 Pavel Feldman 2013-01-24 01:15:24 PST
Comment on attachment 184420 [details]
Screenshot of the feature

I'd move hint box one pixel up and two pixels to the left + bump "i" 1 pixel up. You might also want to add text shadow for it.
Comment 5 Sergey Ryazanov 2013-01-24 01:53:22 PST
Created attachment 184442 [details]
Patch
Comment 6 Sergey Ryazanov 2013-01-24 01:56:27 PST
Created attachment 184444 [details]
Updated screenshot
Comment 7 Sergey Ryazanov 2013-01-24 02:02:16 PST
(In reply to comment #3)
> (From update of attachment 184419 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=184419&action=review
> 
> > Source/WebCore/inspector/front-end/ConsoleMessage.js:308
> > +        var note = section.element.querySelector(".header > .title").createChild("span", "object-info-state-note");
> 
> You should not reach out for ObjectPropertiesSection internals from here - it is better to have this code in the object properties section itself. You can add ObjectPropertiesSection.prototype.showExpansionHint(enabled) to control it from here, default it to false.

It exposed on the same level as section.element (now I made it clear by removing querySelector) which this code already uses. So I don't think it's violation of encapsulation.

> > Source/WebCore/inspector/front-end/inspector.css:1216
> > +    display: none;
> 
> We try to be minimalistic wrt styles - sounds like alignment ones are not needed here.

Removed vertical-align. text-align is essential.

> > Source/WebCore/inspector/front-end/inspector.css:1224
> > +    font-size: 10px;
> 
> We typically inherit font proeprties.

Didn't get what are you suggesting. Inherited value is 11px. So I need make font smaller or enlarge the box.
Comment 8 Pavel Feldman 2013-01-24 05:05:37 PST
Comment on attachment 184442 [details]
Patch

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

Slightly adjusted style below. Otherwise looks good.

> Source/WebCore/inspector/front-end/inspector.css:1215
> +.object-info-state-note {

Here is my version (round balloon):

width: 12px;
height: 12px;
background-color: rgb(179, 203, 247);
color: white;
text-align: center;
border-radius: 5px;
line-height: 13px;
margin: 0 8px;
Comment 9 WebKit Review Bot 2013-01-24 14:53:17 PST
Comment on attachment 184442 [details]
Patch

Attachment 184442 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16080877

New failing tests:
inspector/console/console-tests.html
inspector/console/console-log-document-proto.html
inspector/console/console-external-array.html
inspector/console/console-log-toString-object.html
inspector/console/console-big-array.html
inspector/console/console-eval-syntax-error.html
inspector/console/console-message-format.html
inspector/console/console-format.html
inspector/console/console-object-constructor-name.html
inspector/console/console-dir.html
inspector/console/console-eval-undefined-override.html
inspector/console/console-dirxml.html
inspector/console/console-object-preview.html
inspector/console/console-format-collections.html
Comment 10 Sergey Ryazanov 2013-01-24 22:38:40 PST
Created attachment 184669 [details]
Patch
Comment 11 Sergey Ryazanov 2013-01-24 22:39:53 PST
Created attachment 184670 [details]
Updated screenshot
Comment 12 WebKit Review Bot 2013-01-24 22:56:48 PST
Comment on attachment 184669 [details]
Patch

Rejecting attachment 184669 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-03', 'apply-attachment', '--no-update', '--non-interactive', 184669, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
webkit-commit-queue

Parsed 4 diffs from patch file(s).
patch: **** Can't create file /tmp/ppSAsOFf : No space left on device
patch: **** Can't create file /tmp/ppZm1GVf : No space left on device
patch: **** Can't create file /tmp/ppkY8lre : No space left on device
patch: **** Can't create file /tmp/ppEgfH4d : No space left on device

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force', '--reviewer', 'Pavel Feldman']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/16122141
Comment 13 WebKit Review Bot 2013-01-24 23:00:26 PST
Comment on attachment 184669 [details]
Patch

Rejecting attachment 184669 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-03', 'apply-attachment', '--no-update', '--non-interactive', 184669, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
webkit-commit-queue

Parsed 4 diffs from patch file(s).
patch: **** Can't create file /tmp/ppWivw1O : No space left on device
patch: **** Can't create file /tmp/ppMKFNdP : No space left on device
patch: **** Can't create file /tmp/ppgPhXRO : No space left on device
patch: **** Can't create file /tmp/ppQEj7iN : No space left on device

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force', '--reviewer', 'Pavel Feldman']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/16123124
Comment 14 WebKit Review Bot 2013-01-25 03:17:24 PST
Comment on attachment 184669 [details]
Patch

Rejecting attachment 184669 [details] from commit-queue.

New failing tests:
inspector/console/console-tests.html
inspector/console/console-log-document-proto.html
inspector/console/console-external-array.html
inspector/console/console-log-toString-object.html
inspector/console/console-big-array.html
inspector/console/console-eval-syntax-error.html
inspector/console/console-message-format.html
inspector/console/console-format.html
inspector/console/console-object-constructor-name.html
inspector/console/console-dir.html
inspector/console/console-eval-undefined-override.html
inspector/console/console-dirxml.html
inspector/console/console-object-preview.html
inspector/console/console-format-collections.html
Full output: http://queues.webkit.org/results/16120230
Comment 15 Sergey Ryazanov 2013-01-25 03:55:51 PST
Created attachment 184718 [details]
Patch
Comment 16 WebKit Review Bot 2013-01-25 04:37:32 PST
Comment on attachment 184718 [details]
Patch

Clearing flags on attachment: 184718

Committed r140813: <http://trac.webkit.org/changeset/140813>
Comment 17 WebKit Review Bot 2013-01-25 04:37:37 PST
All reviewed patches have been landed.  Closing bug.