[webkit-reviews] review denied: [Bug 27613] WebCore/page/ContextMenuController.cpp does not conform to style standards : [Attachment 33350] Style patch for ContextMenuController.cpp

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has denied Mike Fenton
<mike.fenton at torchmobile.com>'s request for review:
Bug 27613: WebCore/page/ContextMenuController.cpp does not conform to style
standards
https://bugs.webkit.org/show_bug.cgi?id=27613

Attachment 33350: Style patch for ContextMenuController.cpp
https://bugs.webkit.org/attachment.cgi?id=33350&action=review

------- Additional Comments from David Levin <levin at chromium.org>
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> +2009-07-23  Mike Fenton  <mike.fenton at torchmobile.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Update WebCore/page/ContextMenuController.cpp to conform to WebKit
> +	   Style Guidelines as identified by cpplint.py.

Now it is cpp_style.py :)


> diff --git a/WebCore/page/ContextMenuController.cpp
b/WebCore/page/ContextMenuController.cpp


Since this is about fixing styling in ContextMenuController.cpp,


Why not fix the indentation in this block of code?  Personally, I would unwrap
this line but you could just indent to the createWindow(.


107	static void openNewWindow(const KURL& urlToLoad, Frame* frame)
108	{
109	    if (Page* oldPage = frame->page()) {
110		WindowFeatures features;
111		if (Page* newPage = oldPage->chrome()->createWindow(frame,
112			FrameLoadRequest(ResourceRequest(urlToLoad,
frame->loader()->outgoingReferrer())), features))
113		    newPage->chrome()->show();
114	    }
115	}


> +    case ContextMenuItemTagOpenFrameInNewWindow: {
> +	   DocumentLoader* loader = frame->loader()->documentLoader();
> +	   if (!loader->unreachableURL().isEmpty())
> +	       openNewWindow(loader->unreachableURL(), frame);
> +	   else
> +	       openNewWindow(loader->url(), frame);
> +	   break;
>	   }
The indentation for the "}" needs fixing.

> +    case ContextMenuItemTagSpellingGuess:
> +	   ASSERT(frame->selectedText().length());
> +	   if (frame->editor()->shouldInsertText(item->title(),
frame->selection()->toNormalizedRange().get(),
> +	       EditorInsertActionPasted)) {
Either indent to the level of the nested paren or unwrap.

> +	       Document* document = frame->document();
> +	       RefPtr<ReplaceSelectionCommand> command =
> +		   ReplaceSelectionCommand::create(document,
createFragmentFromMarkup(document, item->title(), ""),
> +									       
  true, false, true);

Either indent to the level of the nested paren or unwrap.


> +    case ContextMenuItemTagOpenLink:
> +	   if (Frame* targetFrame = result.targetFrame()) {
> +	      
targetFrame->loader()->loadFrameRequest(FrameLoadRequest(ResourceRequest(result
.absoluteLinkURL(),
> +		   frame->loader()->outgoingReferrer())), false, false, 0, 0);

Either indent to the level of the nested paren or unwrap.