Bug 68342

Summary:

Web Inspector: streamline Console's MessageType and MessageSource semantics.

Product:

WebKit

Reporter:

Pavel Feldman <pfeldman>

Component:

Web Inspector (Deprecated)

Assignee:

Pavel Feldman <pfeldman>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, vsevik, yurys

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
[Path] Mac bot fixed.	none
[Patch] Review comments addressed	none
[Patch] rebaselined 2.	none
[Patch] rebaselined 3.	tonyg: review+

Description Pavel Feldman 2011-09-19 02:09:46 PDT

This change fixes semantics of the MessageSource and MessageType:
MessageSource is now the source of the message (be it Network, HTML parser or Console API)
MessageType is only defined for the Console API messages and contains the name of the API call (log, dir, dirxml, etc.).

Subsequent https://bugs.webkit.org/show_bug.cgi?id=66371 will make MessageType private to the inspector.

Comment 1 Pavel Feldman 2011-09-19 02:56:10 PDT

Created attachment 107825 [details]
Patch

Comment 2 WebKit Review Bot 2011-09-19 07:51:15 PDT

Comment on attachment 107825 [details]
Patch

Attachment 107825 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/9744296

Comment 3 Pavel Feldman 2011-09-19 08:28:25 PDT

Created attachment 107858 [details]
[Path] Mac bot fixed.

Comment 4 Vsevolod Vlasov 2011-09-19 09:46:21 PDT

Comment on attachment 107858 [details]
[Path] Mac bot fixed.

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

> Source/WebCore/inspector/front-end/ConsoleMessage.js:98
> +            case WebInspector.ConsoleMessage.MessageType.Dir:

We should not switch between MessageTypes unless MessageSource is ConsoleAPI.
Please put switch behind a conditional operator and make MessageType optional in protocol.

Otherwise looks good.

Comment 5 Pavel Feldman 2011-09-19 10:39:36 PDT

Created attachment 107879 [details]
[Patch] Review comments addressed

Comment 6 Pavel Feldman 2011-09-19 10:53:34 PDT

Created attachment 107884 [details]
[Patch] rebaselined 2.

Comment 7 Pavel Feldman 2011-09-19 10:55:25 PDT

Created attachment 107886 [details]
[Patch] rebaselined 3.

Comment 8 Vsevolod Vlasov 2011-09-20 04:52:18 PDT

LGTM

Comment 9 Tony Gentilcore 2011-09-20 05:42:22 PDT

Comment on attachment 107886 [details]
[Patch] rebaselined 3.

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

> Source/WebCore/inspector/Inspector.json:335
> +        "description": "Console domain defines methods and events for interaction with the JavaScript console. Console collects messages created by means of the <a href='http://getfirebug.com/wiki/index.php/Console_API'>JavaScript Console API</a>. One needs to enable this domain using <code>enable</code> command in order to start receiving the console messages. Browser collects messages issued while console domain is not enabled as well and reports them using <code>messageAdded</code> notification upon enabling.",

I take it these strings are localized so it is fine to change them in place?

Comment 10 Pavel Feldman 2011-09-20 05:56:09 PDT

Comment on attachment 107886 [details]
[Patch] rebaselined 3.

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

>> Source/WebCore/inspector/Inspector.json:335
>> +        "description": "Console domain defines methods and events for interaction with the JavaScript console. Console collects messages created by means of the <a href='http://getfirebug.com/wiki/index.php/Console_API'>JavaScript Console API</a>. One needs to enable this domain using <code>enable</code> command in order to start receiving the console messages. Browser collects messages issued while console domain is not enabled as well and reports them using <code>messageAdded</code> notification upon enabling.",
> 
> I take it these strings are localized so it is fine to change them in place?

Protocol description is not localized.

Comment 11 Pavel Feldman 2011-09-20 06:45:17 PDT

Committed r95535: <http://trac.webkit.org/changeset/95535>