Bug 35801 - Web Inspector: generate preview for the objects dumped into the console upon logging.
: Web Inspector: generate preview for the objects dumped into the console upon ...
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated)
: 528+ (Nightly build)
: Macintosh Mac OS X 10.6
: P2 Normal
Assigned To: Pavel Feldman
:
Depends on: 93505 93511 93512
Blocks:
  Show dependency treegraph
 
Reported: 2010-03-05 11:37 PST by mitch kramer
Modified: 2012-12-01 13:36 PST (History)
13 users (show)

See Also:


Attachments
Patch (75.96 KB, patch)
2012-08-07 08:46 PDT, Pavel Feldman
no flags Details | Formatted Diff | Diff
Patch (64.50 KB, patch)
2012-08-08 06:40 PDT, Pavel Feldman
no flags Details | Formatted Diff | Diff
Patch (64.60 KB, patch)
2012-08-08 06:55 PDT, Pavel Feldman
no flags Details | Formatted Diff | Diff
Patch (64.46 KB, patch)
2012-08-08 08:37 PDT, Pavel Feldman
vsevik: review+
buildbot: commit‑queue-
Details | Formatted Diff | Diff
Assertion backtrace (2.20 KB, text/plain)
2012-08-08 11:49 PDT, Thiago Marcos P. Santos
no flags Details
Screenshot showing that the length of an array is inconsistent. (12.98 KB, image/png)
2012-12-01 13:36 PST, Rohit Banga
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description mitch kramer 2010-03-05 11:37:45 PST
1) create an object literal with one or more properties
2) console.log that object but leave it closed (don't expand it in the console)
3) change one of the properties to a new value

now open that console.log and you'll see it has the new value for some reason, even though it's value was different at the time it was generated.
Comment 1 mitch kramer 2010-03-05 11:50:19 PST
I should point out that if you open it, it will retain the correct value if that wasn't clear.
Comment 2 Pavel Feldman 2010-03-09 06:33:36 PST
I don't think we are ever going to fix this one. We can't clone object upon dumping it into the console and we also can't listen to the object properties' changes in order to make it always actual.

We should make sure existing behavior is expected though.
Comment 3 mitch kramer 2010-03-17 07:22:20 PDT
If i'm "logging it" It would seem that i would be taking a snapshot of it at that time... It seems broken to me to not have it be historical -- but if you can't do it, i guess you can't do it... just seems like it might be FAR more useful if it worked as I expected it to.
Comment 4 mitch kramer 2010-03-18 13:48:24 PDT
Thinking about this further, it's odd that the behavior is different depending on whether or not you've expanded it or not... perhaps the fix is to have it expanded by default, since that keeps the correct data then. Chances are pretty good you're going to want to see that data anyway since you're console.logging it. Or maybe just perform those same operations in the background and not update the UI...
Comment 5 Pavel Feldman 2010-03-18 13:55:51 PDT
(In reply to comment #4)
> Thinking about this further, it's odd that the behavior is different depending
> on whether or not you've expanded it or not... perhaps the fix is to have it
> expanded by default, since that keeps the correct data then. Chances are pretty
> good you're going to want to see that data anyway since you're console.logging
> it. Or maybe just perform those same operations in the background and not
> update the UI...

What if the inspected value is on the second level? In either case, for debugging purposes one can always use toString or JSON.stringify.
Comment 6 mitch kramer 2010-03-19 14:01:41 PDT
(In reply to comment #5)
> What if the inspected value is on the second level? In either case, for
> debugging purposes one can always use toString or JSON.stringify.

That's fair enough, this behavior in general still feels wrong to me though. But if I stand alone in that, well, then, I guess I stand alone.
Comment 7 André Hänsel 2011-05-28 19:27:19 PDT
(In reply to comment #2)
> I don't think we are ever going to fix this one. We can't clone object upon dumping it into the console and we also can't listen to the object properties' changes in order to make it always actual.

Why Firefox/Firebug can?

What use does the functionality of dumping objects have other than seeing the object *in the state it was in when being logged*?
Comment 8 Quentin 2011-11-04 05:59:48 PDT
I agree with André.
This can lead to very strange log behaviour when your are not aware of this problem, such as http://stackoverflow.com/questions/8007397/javascript-constructor-with-google-chrome-bugged
Comment 9 Nick S 2012-02-10 13:05:00 PST
Here is another example of unexpected output for console.log()

http://jsfiddle.net/MeuXt/2/

You can see that toString() makes it output the correct value, but if you were to run that code without the toString(), you'd think that the array was somehow magically updated before the code was run on it.
Comment 10 Timothy Hatcher 2012-03-19 06:49:02 PDT
*** Bug 75561 has been marked as a duplicate of this bug. ***
Comment 11 Rohit Banga 2012-07-27 04:09:33 PDT
I saw a problem today which I suspect is related to this bug.

   eventHandler1: function(arr) {
      this.mainArr.push(arr);
      console.log(this.mainArr);    // prints correct array
   }

   eventHandler2: function() {
      console.log(this.mainArr);    // prints [undefined x 1]    ... WAT!!!
      var arr = this.mainArr.pop();
      // use and get rid of arr
   }

 
eventHandler2 is invoked a few seconds after eventHandler1.
Because of the pop() call the value of mainArr is printed as [undefined x 1] in eventHandler2. If I just comment the call to pop() then the value is printed correctly. It seems console.log() is resolving the reference after it has been GC'ed.

I also tried to invoke eventHandler2 immediately after eventHandler1 and managed to generate [undefined x 1] for both console.log() statements. So I would vaguely describe it as a race condition between GC and console renderer.

PS: Its 4 in the morning and I am too lazy to write a jsfiddle.
Comment 12 Paul Irish 2012-07-30 06:48:40 PDT
Downstream ticket: http://crbug.com/50316
Comment 13 Paul Irish 2012-08-02 07:47:01 PDT
*** Bug 49501 has been marked as a duplicate of this bug. ***
Comment 14 Pavel Feldman 2012-08-07 08:46:07 PDT
Created attachment 156949 [details]
Patch
Comment 15 Vsevolod Vlasov 2012-08-08 00:37:58 PDT
Comment on attachment 156949 [details]
Patch

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

> Source/WebCore/inspector/InjectedScriptSource.js:714
> +    /** @param {!Object} object*/

nit: Please add new lines

> Source/WebCore/inspector/front-end/ConsoleMessage.js:322
> +            if (isArray && name === "length")

Why not skip it when creating an abbreviation?

> Source/WebCore/inspector/front-end/ConsoleMessage.js:327
> +

extra line

> Source/WebCore/inspector/front-end/ConsoleMessage.js:329
> +            if (!isArray || name != i) {

!==
When is this needed?

> Source/WebCore/inspector/front-end/ConsoleMessage.js:345
> +                span.textContent = "\"" + value.replace(/\n/g, "\u21B5") + "\"";

I think trimmed string should also cause isLosslessAbbreviation = false;

> Source/WebCore/inspector/front-end/ConsoleView.js:725
>                  return;

Please revert ConsoleView changes, as they were reviewed in another bug.
Comment 16 Pavel Feldman 2012-08-08 06:40:34 PDT
Created attachment 157204 [details]
Patch
Comment 17 Andrey Kosyakov 2012-08-08 06:49:30 PDT
Comment on attachment 157204 [details]
Patch

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

> Source/WebCore/inspector/InjectedScriptSource.js:720
> +        var previewPropertyCount = 0;

where is this used?
Comment 18 Andrey Kosyakov 2012-08-08 06:54:14 PDT
Comment on attachment 157204 [details]
Patch

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

> Source/WebCore/inspector/front-end/ConsoleMessage.js:313
> +              isLosslessPreview = false;

indent

> Source/WebCore/inspector/front-end/ConsoleMessage.js:344
> +            if (type === "object") {
> +                var subtype = property[3];
> +                if (subtype === "node")
> +                    span.addStyleClass("console-formatted-preview-node");
> +                span.textContent = value;
> +            } else if (type === "string")
> +                span.textContent = "\"" + value.replace(/\n/g, "\u21B5") + "\"";
> +            else if (type === "null" || type === "undefined")
> +                span.textContent = type;
> +            else
> +                span.textContent = String(value);

switch (type)?
Comment 19 Pavel Feldman 2012-08-08 06:55:21 PDT
Created attachment 157208 [details]
Patch
Comment 20 Build Bot 2012-08-08 07:25:13 PDT
Comment on attachment 157208 [details]
Patch

Attachment 157208 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13449848
Comment 21 Andrey Kosyakov 2012-08-08 07:55:06 PDT
Comment on attachment 157208 [details]
Patch

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

> Source/WebCore/inspector/InjectedScriptSource.js:775
> +            if (!preview.length)
> +                return undefined;

So what happens to empty arrays?

> Source/WebCore/inspector/front-end/ConsoleMessage.js:340
> +            } else if (type === "string")
> +                span.textContent = "\"" + value.replace(/\n/g, "\u21B5") + "\"";

This would turn 'a"b' into "a"b" which would looks a bit weird. I think if we're adding outer quotes, we should escape string to make it parseable.
Comment 22 Pavel Feldman 2012-08-08 08:37:59 PDT
Created attachment 157228 [details]
Patch
Comment 23 Vsevolod Vlasov 2012-08-08 09:10:40 PDT
Comment on attachment 157228 [details]
Patch

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

> Source/WebCore/inspector/InjectedScriptSource.js:641
> +            return obj.nodeType === 1 /* Node.ELEMENT_NODE */ ? "<" + obj.nodeName.toLowerCase() + ">" : obj.nodeName.toLowerCase();

doctype?

> Source/WebCore/inspector/InjectedScriptSource.js:734
> +                    overflow = true;

Please add lossless = false

> Source/WebCore/inspector/InjectedScriptSource.js:753
> +                const maxLength = 20;

100?

> LayoutTests/inspector/console/console-format-expected.txt:58
> +[/^url\(\s*(?:(?:"(?:…] console-format.html:55

So regexps preview is lossy, we should add a tree view for such an object

> LayoutTests/inspector/console/console-format-expected.txt:77
> +<body onload="onload()"> console-format.html:54

This change looks somewhat unexpected for me. Did you intend to change the way elements are printed?

> LayoutTests/inspector/console/console-format.html:17
> +    array.foo = {}; 

Why did this change?
Comment 24 Vsevolod Vlasov 2012-08-08 09:11:17 PDT
Comment on attachment 157228 [details]
Patch

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

> LayoutTests/inspector/console/console-format-expected.txt:122
> +id="x""x"

I don't understand what is going on here. :(
Comment 25 Build Bot 2012-08-08 09:12:02 PDT
Comment on attachment 157228 [details]
Patch

Attachment 157228 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13454538
Comment 26 Pavel Feldman 2012-08-08 10:22:44 PDT
Committed r125046: <http://trac.webkit.org/changeset/125046>
Comment 27 Thiago Marcos P. Santos 2012-08-08 11:49:25 PDT
Created attachment 157264 [details]
Assertion backtrace

The EFL Debug bot is hitting this assertion after this patch.
Comment 28 Pavel Feldman 2012-08-08 12:04:35 PDT
I'll roll it out.
Comment 29 WebKit Review Bot 2012-08-08 12:13:44 PDT
Re-opened since this is blocked by 93505
Comment 30 Pavel Feldman 2012-08-09 10:51:38 PDT
Landed as http://trac.webkit.org/changeset/125174
Comment 31 Pieter Wigboldus 2012-09-05 07:54:56 PDT
Please check e.g. this in your console:

var s = ["hi"];
console.log(s);
s[0] = "bye";
console.log(s);

it show 2 times bye.
can be solved with 
console.log(s.toString());
or
console.log(s.slice());

In Firebug it show values that you expect, Chrome/Webkit show 2 times the last value.
Comment 32 Pavel Feldman 2012-09-06 02:28:16 PDT
That's what this patch fixed.
Comment 33 Rohit Banga 2012-12-01 13:36:42 PST
Created attachment 177112 [details]
Screenshot showing that the length of an array is inconsistent.

I have an array called tabContent. I dump it to the console and its length. The length is now 1. The object dumped is not expanded initially.
Then I add another element to the array. Again print the same array.
Now when I expand both the first one shows the two elements inside when at the time of printing there was only one. Also the length, preview (unexpanded array) is inconsistent with what we see in the expanded view.

I am using Chrome Version 24.0.1312.27 beta.