[Webkit-unassigned] [Bug 26988] Haiku-specific files for WebCore

[bugzilla-daemon at webkit.org]

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


Darin Adler <darin at apple.com> changed:

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




--- Comment #50 from Darin Adler <darin at apple.com>  2009-08-07 07:42:26 PDT ---
(From update of attachment 34263)
> +#include "DocumentFragment.h"
> +#include "Editor.h"
> +#include "Frame.h"
> +#include "KURL.h"
> +#include "NotImplemented.h"
> +#include "markup.h"
> +
> +#include <support/Locker.h>
> +#include <Clipboard.h>
> +#include <Message.h>
> +#include <String.h>

WebKit style is to use a single paragraph for these, not two.

> +#include "KeyboardCodes.h"
> +#include "NotImplemented.h"
> +
> +#include <InterfaceDefs.h>
> +#include <Message.h>
> +#include <String.h>

Ditto.

> +static String keyIdentifierForHaikuKeyCode(char singleByte, int32 keyCode)

Are you sure you want to use the type "int32" here? We'd normally use int, or
perhaps int32_t. Is this something specific to the Haiku platform?

> +{
> +        switch (singleByte) {

This is indented two many spaces.

> +                return String::format("U+%04X", toASCIIUpper((int)keyCode));

You should not have to use any type cast with toASCIIUpper. Does this help in
some way?

> +            };

Extra semicolon here.

> +            return String::format("U+%04X", toASCIIUpper((int)keyCode));

If you put this after the switch statement then you could share the two cases
of it.

> +    switch (singleByte) {

For some reason this second switch statement is indented differently than the
first. Could you make both consistent, and follow the WebKit coding style
guide?

> +PlatformKeyboardEvent::PlatformKeyboardEvent(BMessage* message)
> +{
> +    BString bytes = message->FindString("bytes");
> +
> +    m_text = String::fromUTF8(bytes.String(), bytes.Length());
> +    m_unmodifiedText = String(bytes.String(), bytes.Length());
> +    m_keyIdentifier = keyIdentifierForHaikuKeyCode(bytes.ByteAt(0), message->FindInt32("key"));
> +
> +    if (message->what == B_KEY_UP)
> +        m_type = KeyUp;
> +    else if (message->what == B_KEY_DOWN)
> +        m_type = KeyDown;
> +
> +    m_autoRepeat = true;
> +    m_ctrlKey = false;
> +    m_altKey = false;
> +    m_metaKey = false;
> +    m_windowsVirtualKeyCode = windowsKeyCodeForKeyEvent(bytes.ByteAt(0));
> +    m_isKeypad = false;
> +    m_shiftKey = false;
> +}

It's better style to use member initialization as much as possible, and only
use assignment when that is impractical. So at least for most of these booleans
you should switch to that.

> +PlatformMouseEvent::PlatformMouseEvent(const BMessage* message)

Same comment about member-wise initialization for this constructor.

> +    m_deltaX *= -static_cast<float>(cScrollbarPixelsPerLineStep);
> +    m_deltaY *= -static_cast<float>(cScrollbarPixelsPerLineStep);

Is the cast to float needed? What effect does it have? I'd expect this to work
perfectly without the cast.

review- because there are enough comments above that could be simply addressed

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