Bug 48408

Summary: Port ContextMenuWin.cpp to WinCE
Product: WebKit Reporter: Patrick R. Gansterer <paroga>
Component: PlatformAssignee: Patrick R. Gansterer <paroga>
Status: RESOLVED FIXED    
Severity: Normal CC: aroben, commit-queue
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: Other   
Attachments:
Description Flags
Patch
aroben: review-, aroben: commit-queue-
Patch none

Description Patrick R. Gansterer 2010-10-27 03:01:30 PDT
see patch
Comment 1 Patrick R. Gansterer 2010-11-19 11:54:14 PST
Created attachment 74411 [details]
Patch
Comment 2 Adam Roben (:aroben) 2010-11-22 11:31:23 PST
Comment on attachment 74411 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=74411&action=review

All the changes of HMENU -> PlatformMenuDescription don't really seem helpful. At the very least they seem unrelated to the rest of the patch.

> WebCore/platform/win/ContextMenuWin.cpp:118
> +    UINT flags = MF_BYPOSITION;
> +    UINT newItem = 0;
> +    LPCWSTR title = 0;
> +
> +    if (item.type() == SeparatorType)
> +        flags |= MF_SEPARATOR;
> +    else {
> +        flags |= MF_STRING;
> +        flags |= item.checked() ? MF_CHECKED : MF_UNCHECKED;
> +        flags |= item.enabled() ? MF_ENABLED : MF_GRAYED;
> +
> +        PlatformMenuItemDescription description = item.releasePlatformDescription();
> +        title = description->dwTypeData;
> +        description->dwTypeData = 0;
> +
> +        if (description->hSubMenu) {
> +            flags |= MF_POPUP;
> +            newItem = reinterpret_cast<UINT>(description->hSubMenu);
> +            description->hSubMenu = 0;
> +        } else
> +            newItem = description->wID;
> +
> +        free(description);
> +    }
> +
> +    if (::InsertMenuW(m_platformDescription, position, flags, newItem, title))
> +        ++m_itemCount;

I think it would be clearer if this code were pulled out into a separate function.

> WebCore/platform/win/ContextMenuWin.cpp:152
> +#if OS(WINCE)
> +    UINT type = info->fType & MFT_STRING;
> +#else
>      UINT type = info->fType & ~(MFT_MENUBARBREAK | MFT_MENUBREAK | MFT_OWNERDRAW | MFT_RADIOCHECK | MFT_RIGHTORDER | MFT_RIGHTJUSTIFY);
> +#endif

I think non-CE Windows can use the CE codepath, too.
Comment 3 Patrick R. Gansterer 2010-11-22 11:51:18 PST
Created attachment 74580 [details]
Patch
Comment 4 WebKit Commit Bot 2010-11-22 22:26:42 PST
Comment on attachment 74580 [details]
Patch

Clearing flags on attachment: 74580

Committed r72585: <http://trac.webkit.org/changeset/72585>
Comment 5 WebKit Commit Bot 2010-11-22 22:26:48 PST
All reviewed patches have been landed.  Closing bug.