Bug 48408 - Port ContextMenuWin.cpp to WinCE
Summary: Port ContextMenuWin.cpp to WinCE
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Other
: P2 Normal
Assignee: Patrick R. Gansterer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-27 03:01 PDT by Patrick R. Gansterer
Modified: 2010-11-22 22:26 PST (History)
2 users (show)

See Also:


Attachments
Patch (7.27 KB, patch)
2010-11-19 11:54 PST, Patrick R. Gansterer
aroben: review-
aroben: commit-queue-
Details | Formatted Diff | Diff
Patch (6.33 KB, patch)
2010-11-22 11:51 PST, Patrick R. Gansterer
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.