[Webkit-unassigned] [Bug 27543] Add platform/wince/ files for WINCE port

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=27543


Adam Treat <treat at kde.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #33265|review?                     |review-
               Flag|                            |




--- Comment #2 from Adam Treat <treat at kde.org>  2009-07-22 11:54:38 PDT ---
(From update of attachment 33265)
> +#include "CString.h"
> +#include "DocumentFragment.h"
> +#include "KURL.h"
> +#include "PlatformString.h"
> +#include "TextEncoding.h"
> +#include "markup.h"

Alphabetical sorting problem. Is this a bug in cpplint with capitalization?

> +static String extractURL(const String &inURL, String* title)

Decoration of 'const String &inURL' is wrong.  Another cpplint error?

> +    String url = inURL;
> +    int splitLoc = url.find('\n');
> +    if (splitLoc > 0) {

Minor nit: splitLoc should be spelled out fully: splitLocation.

> +        if (title)
> +            *title = url.substring(splitLoc+1);

Space between splitLoc and 1. Rule #2 of spacing.

> +//Firefox text/html
> +static FORMATETC* texthtmlFormat()
> +{
> +    static UINT cf = RegisterClipboardFormat(L"text/html");
> +    static FORMATETC texthtmlFormat = {cf, 0, DVASPECT_CONTENT, -1, TYMED_HGLOBAL};
> +    return &texthtmlFormat;
> +}

Should be 'textHTMLFormat()' I believe.  What is the Firefox comment for?

> +    HGLOBAL cbData = GlobalAlloc(GPTR, size * sizeof(UChar));

Should be clipboardData or so.

> +    // calculate offsets
> +    const unsigned UINT_MAXdigits = 10; // number of digits in UINT_MAX in base 10
> +    unsigned startHTMLOffset = cf_html.length() + startHTML.length() + endHTML.length() + startFragment.length() + endFragment.length() + (shouldFillSourceURL ? sourceURL.length() + srcURL.length() : 0) + (4*UINT_MAXdigits);

Space around '4*UINT_MAXdigits'.

> +String getURL(IDataObject* dataObject, bool& success, String* title)
> +{
> +    return String();
> +}

Compiler doesn't complain of unused arguments?

> +#endif // ClipboardWin_h

Should be // ClipboardWince_h I think.

> +void ContextMenuItem::setSubMenu(ContextMenu* subMenu)
> +{
> +    if (m_platformDescription) {
> +        if (m_platformDescription->subMenu != subMenu->platformDescription())
> +            delete m_platformDescription->subMenu;
> +        m_platformDescription->subMenu = subMenu->releasePlatformDescription();
> +    }
> +}

Early return please.

> +void ContextMenuItem::setChecked(bool checked)
> +{
> +    if (m_platformDescription) {
> +        if (checked)
> +            m_platformDescription->state |= PlatformMenuItemDescriptionType::CheckedState;
> +        else
> +            m_platformDescription->state &= ~PlatformMenuItemDescriptionType::CheckedState;
> +    }
> +}

Early return please.

> +void ContextMenuItem::setEnabled(bool enabled)
> +{
> +    if (m_platformDescription) {
> +        if (enabled)
> +            m_platformDescription->state |= PlatformMenuItemDescriptionType::EnabledState;
> +        else
> +            m_platformDescription->state &= ~PlatformMenuItemDescriptionType::EnabledState;
> +    }
> +}

Early return please.

> +PlatformMenuDescriptionType::PlatformMenuDescriptionType()
> +: hMenu(::CreatePopupMenu())
> +, itemCount(0)
> +{
> +}

Needs indentation of member variable initialization.

> +    if (desc->type == SeparatorType) {
> +        flags |= MF_SEPARATOR;
> +    } else {

One line if clause...

> +#if 0
> +    if (!m_platformDescription)
> +        return;
> +
> +    // FIXME: the Windows version did this, is it necessary?
> +    MENUINFO menuInfo = {0};
> +    menuInfo.cbSize = sizeof(MENUINFO);
> +    menuInfo.fMask = MIM_STYLE;
> +    ::GetMenuInfo(m_platformDescription, &menuInfo);
> +    menuInfo.fMask = MIM_STYLE;
> +    menuInfo.dwStyle |= MNS_NOTIFYBYPOS;
> +    ::SetMenuInfo(m_platformDescription, &menuInfo);
> +#endif

--commented out code

Going to stop there.  For the next iteration can you break it up in the
following way?

Patch 1:


 * platform/wince/ClipboardUtilitiesWince.cpp: Added.
 * platform/wince/ClipboardWince.cpp: Added.
 * platform/wince/ClipboardWince.h: Added.
 * platform/wince/ContextMenuItemWince.cpp: Added.
 * platform/wince/ContextMenuWince.cpp: Added.
 * platform/wince/CursorWince.cpp: Added.
 * platform/wince/DragDataWince.cpp: Added.
 * platform/wince/DragImageWince.cpp: Added.
 * platform/wince/EditorWince.cpp: Added.
 * platform/wince/FileChooserWince.cpp: Added.
 * platform/wince/FileSystemWince.cpp: Added.
 * platform/wince/KURLWince.cpp: Added.
 * platform/wince/KeygenWince.cpp: Added.
 * platform/wince/MIMETypeRegistryWince.cpp: Added.
 * platform/wince/OptimizedBuffer.cpp: Added.
 * platform/wince/OptimizedBuffer.h: Added.
 * platform/wince/PasteboardWince.cpp: Added. 
 * platform/wince/SearchPopupMenuWince.cpp: Added.
 * platform/wince/SharedBuffer.cpp: Added.
 * platform/wince/SharedTimerWince.cpp: Added.
 * platform/wince/SystemTimeWince.cpp: Added. 

Patch 2:

The rest of the ScrollView/Widget related files?

Thanks,
Adam
I Indent as above. t

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.