[webkit-reviews] review denied: [Bug 50514] WebKit2: Context menu support on Windows : [Attachment 75603] [PATCH] Context Menu Refactoring w/ style fixes

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Brian Weinstein
<bweinstein at apple.com>'s request for review:
Bug 50514: WebKit2: Context menu support on Windows
https://bugs.webkit.org/show_bug.cgi?id=50514

Attachment 75603: [PATCH] Context Menu Refactoring w/ style fixes 
https://bugs.webkit.org/attachment.cgi?id=75603&action=review

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

I didn’t have time to review this whole thing, but there is a layering
violation here.

> WebCore/platform/ContextMenu.h:55
>	   ContextMenu(const HitTestResult&, const PlatformMenuDescription);
> +#if PLATFORM(WIN)
> +	   ContextMenu(const HitTestResult&, const HMENU);
> +#endif

The “const HMENU” here is no different from HMENU, and the const should be
removed.

Same is true for “const PlatformMenuDescription”.

> WebCore/platform/ContextMenuItem.h:44
> +#include "HitTestResult.h"

This can’t be right. The platform directory is not allowed to include headers
from the rest of WebCore. Something needs to be moved instead.

> WebCore/platform/ContextMenuItem.h:281
> +	   ContextMenuItem(const HitTestResult&, const LPMENUITEMINFO);

This is not acceptable layering. HitTestResult is a browser level concept that
does not belong here at the platform layer.