<html>
    <head>
      <base href="https://bugs.webkit.org/">
    </head>
    <body>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - [WinCairo] Add WebKit Shared/win event files for wincairo webkit"
   href="https://bugs.webkit.org/show_bug.cgi?id=183043#c7">Comment # 7</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - [WinCairo] Add WebKit Shared/win event files for wincairo webkit"
   href="https://bugs.webkit.org/show_bug.cgi?id=183043">bug 183043</a>
              from <span class="vcard"><a class="email" href="mailto:rniwa@webkit.org" title="Ryosuke Niwa <rniwa@webkit.org>"> <span class="fn">Ryosuke Niwa</span></a>
</span></b>
        <pre>Comment on <span class=""><a href="attachment.cgi?id=334862&action=diff" name="attach_334862" title="Patch">attachment 334862</a> <a href="attachment.cgi?id=334862&action=edit" title="Patch">[details]</a></span>
Patch

View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=334862&action=review">https://bugs.webkit.org/attachment.cgi?id=334862&action=review</a>

r=me as an API owner as well.

<span class="quote">> Source/WebKit/Shared/NativeWebWheelEvent.h:81
> +    MSG m_nativeEvent;</span >

You probably want to initialize MSG here.

<span class="quote">> Source/WebKit/Shared/win/WebEventFactory.cpp:110
> +    if (::GetKeyState(VK_MENU) & 0x8000)</span >

Instead of keep having & 0x8000 everywhere,
can we add an inline helper function like IsKeyInDownState(VM_MENU) / IsKeyStateDown(::GetKeyState(VK_MENU))?

<span class="quote">> Source/WebKit/Shared/win/WebEventFactory.cpp:123
> +    if (::GetKeyState(VK_CONTROL) & 0x8000)
> +        modifiers |= WebEvent::ControlKey;
> +    if (::GetKeyState(VK_SHIFT) & 0x8000)
> +        modifiers |= WebEvent::ShiftKey;
> +    if (::GetKeyState(VK_MENU) & 0x8000)
> +        modifiers |= WebEvent::AltKey;</span >

Ditto about all these checks.

<span class="quote">> Source/WebKit/Shared/win/WebEventFactory.cpp:456
> +    WebEvent::Type type             = keyboardEventTypeForEvent(message);
> +    String text                     = textFromEvent(wparam, type);
> +    String unmodifiedText           = unmodifiedTextFromEvent(wparam, type);
> +    String keyIdentifier            = keyIdentifierFromEvent(wparam, type);
> +    int windowsVirtualKeyCode       = static_cast<int>(wparam);
> +    int nativeVirtualKeyCode        = static_cast<int>(wparam);
> +    int macCharCode                 = 0;
> +    bool autoRepeat                 = HIWORD(lparam) & KF_REPEAT;
> +    bool isKeypad                   = isKeypadEvent(wparam, lparam, type);
> +    bool isSystemKey                = isSystemKeyEvent(message);
> +    WebEvent::Modifiers modifiers   = modifiersForCurrentKeyState();</span >

Nit: Wrong indentation style. Never align assignment like this.</pre>
        </div>
      </p>


      <hr>
      <span>You are receiving this mail because:</span>

      <ul>
          <li>You are the assignee for the bug.</li>
      </ul>
    </body>
</html>