Bug 44103 – Web Inspector: [CRASH] Console formatting crashes after cross-domain navigation.

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Pavel Feldman]

- Go to a page with console.log(document) call
- Navigate to a different domain
- Open inspector

[Pavel Feldman]

Created attachment 64583 [details]
[PATCH] Proposed fix.

I wonder if we should try saving the first string entry. I'll think about doing it while you are reviewing the crash fix.

[Yury Semikhatsky]

Comment on attachment 64583 [details]
[PATCH] Proposed fix.

WebCore/inspector/ConsoleMessage.cpp:142
 +                  inspectorValue = InspectorString::create("(frame collected)");
This string should be localized and live in the front-end. And I think it should be frame navigated.

[Pavel Feldman]

Created attachment 64596 [details]
[PATCH] Review comments addressed.

[Pavel Feldman]

Comment on attachment 64596 [details]
[PATCH] Review comments addressed.

Clearing flags on attachment: 64596

Committed r65506: <http://trac.webkit.org/changeset/65506>

[Pavel Feldman]

All reviewed patches have been landed.  Closing bug.

[Joseph Pecoraro]

Comment on attachment 64596 [details]
[PATCH] Review comments addressed.

> diff --git LayoutTests/http/tests/inspector-enabled/resources/console-log-frame-before-navigation.html
> +  console.log("Console message (C) %d with element", 2010, document.documentElement);

The way this console.log would work (one %d, but two arguments) would be to
print the string, then print the element afterwards. The patch's expected
results were as follows:

> -Message: 2010 HTMLHtmlElement
> +Message: Console message (C) %d with element
>  TEST COMPLETE.

So the desired output for the element was the empty string? I'm fine with
that, but what I saw something else in the bugzilla comments.


> +    // Note that localhost is different from the 127.0.0.1 that tests are runnung against. So this navigation
> +    // is in fact cross-domain.

NIT: Typo. "runnung" => "running".

[Pavel Feldman]

(In reply to comment #6)
> (From update of attachment 64596 [details])
> > diff --git LayoutTests/http/tests/inspector-enabled/resources/console-log-frame-before-navigation.html
> > +  console.log("Console message (C) %d with element", 2010, document.documentElement);
> 
> The way this console.log would work (one %d, but two arguments) would be to
> print the string, then print the element afterwards. The patch's expected
> results were as follows:
> 
> > -Message: 2010 HTMLHtmlElement
> > +Message: Console message (C) %d with element
> >  TEST COMPLETE.
> 
> So the desired output for the element was the empty string? I'm fine with
> that, but what I saw something else in the bugzilla comments.
> 
> 

Desired output is only the first string argument. Rationale: we can't format parameters due to missing global object after the navigation. So we ignore all the parameters: in this case both  - integer and element.

> > +    // Note that localhost is different from the 127.0.0.1 that tests are runnung against. So this navigation
> > +    // is in fact cross-domain.
> 
> NIT: Typo. "runnung" => "running".

Thanks. Will fix later.