[Webkit-unassigned] [Bug 183043] [WinCairo] Add WebKit Shared/win event files for wincairo webkit

[bugzilla-daemon at webkit.org]

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

--- Comment #10 from Yousuke Kimoto <Yousuke.Kimoto at sony.com> ---
Thank you for your review.

(In reply to Ryosuke Niwa from comment #7)
> Comment on attachment 334862 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=334862&action=review
> 
> r=me as an API owner as well.
> 
> > Source/WebKit/Shared/NativeWebWheelEvent.h:81
> > +    MSG m_nativeEvent;
> 
> You probably want to initialize MSG here.
Four members of MSG are used, I think MSG confuse many developers. Since those are initialized at construction, I'll fix it as comment #8.

> > Source/WebKit/Shared/win/WebEventFactory.cpp:110
> > +    if (::GetKeyState(VK_MENU) & 0x8000)
> 
> Instead of keep having & 0x8000 everywhere,
> can we add an inline helper function like IsKeyInDownState(VM_MENU) /
> IsKeyStateDown(::GetKeyState(VK_MENU))?
It is a good idea. will do.

> > 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;
> 
> Ditto about all these checks.
will do

> > 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();
> 
> Nit: Wrong indentation style. Never align assignment like this.
I'll fix it.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180305/95ee2bd6/attachment.html>