Bug 27613 – WebCore/page/ContextMenuController.cpp does not conform to style standards

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Mike Fenton]

cpplint reports a number of valid style fixes that should be made to
WebCore/page/ContextMenuController.cpp.

[Mike Fenton]

Created attachment 33350 [details]
Style patch for ContextMenuController.cpp

[Eric Seidel (no email)]

Comment on attachment 33350 [details]
Style patch for ContextMenuController.cpp

This would be much easier to read if we had a white-space ignoring diff.  It's difficult for me to tell that you haven' actually changed any code here. :(

[Mike Fenton]

The main change in the patch is to fix the indentation of the case statements.  It also fixes the include order and a few whitespace at end of line fixes.

[David Levin]

Comment on attachment 33350 [details]
Style patch for ContextMenuController.cpp

> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> [phone]  Mike Fenton  <[e-mail]>
> +
> +        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.

[Mike Fenton]

Created attachment 33714 [details]
Replacement patch for ContextMenuController.cpp style fixes.

Thanks for the review.

This updated patch has all the noted changes corrected.  Functions have been unwrapped and the indentation of the brace corrected.

[David Levin]

Committed as http://trac.webkit.org/changeset/46544