[webkit-reviews] review granted: [Bug 45693] REGRESSION (new parser): Leopard/Tiger Mail <head>/<body> quirk is gone : [Attachment 70557] Patch

Tue Oct 12 16:40:51 PDT 2010

Darin Adler <darin at apple.com> has granted Andy Estes <aestes at apple.com>'s
request for review:
Bug 45693: REGRESSION (new parser): Leopard/Tiger Mail <head>/<body> quirk is
gone
https://bugs.webkit.org/show_bug.cgi?id=45693

Attachment 70557: Patch
https://bugs.webkit.org/attachment.cgi?id=70557&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=70557&action=review

r=me but I’d like to see the style issues fixed

> WebKit/mac/WebView/WebView.mm:648
> ++ (NSString*)_mailQuirksUserScript
> +{
> +    static NSString* mailQuirksScript = nil;
> +    if (!mailQuirksScript) {
> +	   NSString *scriptPath = [[NSBundle bundleForClass:[WebView class]]
pathForResource:@"MailQuirksUserScript" ofType:@"js"];
> +	   mailQuirksScript = [[NSString alloc]
initWithContentsOfFile:scriptPath];
> +    }
> +    return mailQuirksScript;
> +}

The one-time initialization code should not use an if statement. Instead it
should be in a separate function.

> WebKit/mac/WebView/WebView.mm:658
> +    core(self)->group().addUserScriptToWorld(core([WebScriptWorld world]),
> +						[WebView
_mailQuirksUserScript],
> +						KURL(),
> +						0,
> +						0,
> +						InjectAtDocumentEnd,
> +						InjectInAllFrames);

We don’t line up code with parentheses like this. This should be indented
normally and not formatted vertically like this.