[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 #83605|review?                     |review-
               Flag|                            |




--- Comment #13 from Timothy Hatcher <timothy at apple.com>  2011-02-24 09:40:24 PST ---
(From update of attachment 83605)
View in context: https://bugs.webkit.org/attachment.cgi?id=83605&action=review

Looking good. Still some things that should be fixed.

> Source/WebKit/mac/WebCoreSupport/WebChromeClient.mm:104
> +NSString * const ConsoleMessageHTMLMessageSource  = @"HTMLMessageSource";
> +NSString * const ConsoleMessageWMLMessageSource   = @"WMLMessageSource";
> +NSString * const ConsoleMessageXMLMessageSource   = @"XMLMessageSource";
> +NSString * const ConsoleMessageJSMessageSource    = @"JSMessageSource";
> +NSString * const ConsoleMessageCSSMessageSource   = @"CSSMessageSource";
> +NSString * const ConsoleMessageOtherMessageSource = @"OtherMessageSource";
> +
> +NSString * const ConsoleMessageLogMessageType                 = @"LogMessageType";
> +NSString * const ConsoleMessageObjectMessageType              = @"ObjectMessageType";
> +NSString * const ConsoleMessageTraceMessageType               = @"TraceMessageType";
> +NSString * const ConsoleMessageStartGroupMessageType          = @"StartGroupMessageType";
> +NSString * const ConsoleMessageStartGroupCollapsedMessageType = @"StartGroupCollapsedMessageType";
> +NSString * const ConsoleMessageEndGroupMessageType            = @"EndGroupMessageType";
> +NSString * const ConsoleMessageAssertMessageType              = @"AssertMessageType";
> +NSString * const ConsoleMessageUncaughtExceptionMessageType   = @"UncaughtExceptionMessageType";
> +NSString * const ConsoleMessageNetworkErrorMessageType        = @"NetworkErrorMessageType";
> +
> +NSString * const ConsoleMessageTipMessageLevel     = @"TipMessageLevel";
> +NSString * const ConsoleMessageLogMessageLevel     = @"LogMessageLevel";
> +NSString * const ConsoleMessageWarningMessageLevel = @"WarningMessageLevel";
> +NSString * const ConsoleMessageErrorMessageLevel   = @"ErrorMessageLevel";
> +NSString * const ConsoleMessageDebugMessageLevel   = @"DebugMessageLevel";

While const is more correct, none of the other extern NSStrings in WebKit have const. Just drop the const.

> Source/WebKit/mac/WebCoreSupport/WebChromeClient.mm:433
> +        selector = @selector(webView:addMessageToConsole:);
> +        if (![delegate respondsToSelector:selector])
> +            return;
> +        // The old selector only takes JSMessageSource messages.
> +        if (source != JSMessageSource)
> +            return;

You should flip the order here. You shouldn't ask if it responds to the selector unless you plan to call it. That will match the old behavior. (Checking the source first is faster too.)

> Source/WebKit/mac/WebView/WebUIDelegatePrivate.h:119
> +// Message Sources.
> +extern NSString * const ConsoleMessageHTMLMessageSource;
> +extern NSString * const ConsoleMessageWMLMessageSource;
> +extern NSString * const ConsoleMessageXMLMessageSource;
> +extern NSString * const ConsoleMessageJSMessageSource;
> +extern NSString * const ConsoleMessageCSSMessageSource;
> +extern NSString * const ConsoleMessageOtherMessageSource;
> +
> +// Message Types.
> +extern NSString * const ConsoleMessageLogMessageType;
> +extern NSString * const ConsoleMessageObjectMessageType;
> +extern NSString * const ConsoleMessageTraceMessageType;
> +extern NSString * const ConsoleMessageStartGroupMessageType;
> +extern NSString * const ConsoleMessageStartGroupCollapsedMessageType;
> +extern NSString * const ConsoleMessageEndGroupMessageType;
> +extern NSString * const ConsoleMessageAssertMessageType;
> +extern NSString * const ConsoleMessageUncaughtExceptionMessageType;
> +extern NSString * const ConsoleMessageNetworkErrorMessageType;
> +
> +// Message Levels.
> +extern NSString * const ConsoleMessageTipMessageLevel;
> +extern NSString * const ConsoleMessageLogMessageLevel;
> +extern NSString * const ConsoleMessageWarningMessageLevel;
> +extern NSString * const ConsoleMessageErrorMessageLevel;
> +extern NSString * const ConsoleMessageDebugMessageLevel;

Same as above, just drop the const.

> Source/WebKit/mac/WebView/WebUIDelegatePrivate.h:142
> +    @method webView:addMessageToConsole

This is the wrong method name.

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