[Webkit-unassigned] [Bug 54926] All Console Messages should be passed to ChromeClients.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=54926


Timothy Hatcher <timothy at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #83257|review?                     |review-
               Flag|                            |




--- Comment #5 from Timothy Hatcher <timothy at apple.com>  2011-02-22 10:00:32 PST ---
(From update of attachment 83257)
View in context: https://bugs.webkit.org/attachment.cgi?id=83257&action=review

r- for not having extern strings. Dashcode is also a concern that needs considered/tested.

> Source/WebCore/page/Console.cpp:146
> -    if (source == JSMessageSource)
> -        page->chrome()->client()->addMessageToConsole(source, type, level, message, lineNumber, sourceURL);
> +    page->chrome()->client()->addMessageToConsole(source, type, level, message, lineNumber, sourceURL);

The reason we did this was for Dashcode, they were originally the only client of the WebKit API. And when we enhanced the Inspector logging we didn't want to start spamming Dashcode with messages they didn't expect.

Maybe it will be fine, but another approch would be to make a new delegate call and deprecate the old one. The new delegate call gets everything, but the old one only gets JSMessageSource.

> Source/WebKit/mac/WebCoreSupport/WebChromeClient.mm:345
> +    case HTMLMessageSource:
> +        return @"HTMLMessageSource";
> +    case WMLMessageSource:
> +        return @"WMLMessageSource";
> +    case XMLMessageSource:
> +        return @"XMLMessageSource";
> +    case JSMessageSource:
> +        return @"JSMessageSource";
> +    case CSSMessageSource:
> +        return @"CSSMessageSource";
> +    case OtherMessageSource:
> +        return @"OtherMessageSource";

These strings should be defined as "extern NSString *" in the header. And defined in the file with the same symbol.

> Source/WebKit/mac/WebCoreSupport/WebChromeClient.mm:371
> +    case LogMessageType:
> +        return @"LogMessageType";
> +    case ObjectMessageType:
> +        return @"ObjectMessageType";
> +    case TraceMessageType:
> +        return @"TraceMessageType";
> +    case StartGroupMessageType:
> +        return @"StartGroupMessageType";
> +    case StartGroupCollapsedMessageType:
> +        return @"StartGroupCollapsedMessageType";
> +    case EndGroupMessageType:
> +        return @"EndGroupMessageType";
> +    case AssertMessageType:
> +        return @"AssertMessageType";
> +    case UncaughtExceptionMessageType:
> +        return @"UncaughtExceptionMessageType";
> +    case NetworkErrorMessageType:
> +        return @"NetworkErrorMessageType";

These strings too.

> Source/WebKit/mac/WebCoreSupport/WebChromeClient.mm:389
> +    case TipMessageLevel:
> +        return @"TipMessageLevel";
> +    case LogMessageLevel:
> +        return @"LogMessageLevel";
> +    case WarningMessageLevel:
> +        return @"WarningMessageLevel";
> +    case ErrorMessageLevel:
> +        return @"ErrorMessageLevel";
> +    case DebugMessageLevel:
> +        return @"DebugMessageLevel";

These strings too.

> Source/WebKit/mac/WebView/WebUIDelegatePrivate.h:125
> +      <dt>MessageSource</dt> <dd>Where the message came from. HTML, XML, JavaScript, CSS, etc.</dd>
> +      <dt>MessageType</dt> <dd>Class of message. Start / End of a Group, a Log, Network related, etc.</dd>
> +      <dt>MessageLevel</dt> <dd>Severity level of the message. Tip, Log, Warning, etc.</dd>

The comments here should reference the string constants you are going to make.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.