[Webkit-unassigned] [Bug 57904] onClick does not function with <select> elements in WebKit2

[bugzilla-daemon at webkit.org]

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


Anders Carlsson <andersca at apple.com> changed:

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




--- Comment #2 from Anders Carlsson <andersca at apple.com>  2011-04-07 18:21:32 PST ---
(From update of attachment 88727)
View in context: https://bugs.webkit.org/attachment.cgi?id=88727&action=review

> Source/WebKit2/UIProcess/API/mac/WKView.mm:946
> +- (void)Selector:(NSEvent *)theEvent \
> +{ \
> +NativeWebMouseEvent webEvent(theEvent, self); \
> +_data->_page->handleMouseEvent(webEvent); \
> +}

I think we indent the body of multi-line macros.

> Source/WebKit2/UIProcess/WebPageProxy.cpp:782
> +    // (bug #57904) we need to keep track of the mouse down event in the case where we

Please use full URLs when linking to bugs, that makes it easier to open them. Also use a capital letter in the beginning of the sentence.

> Source/WebKit2/UIProcess/WebPageProxy.cpp:785
> +    // when the mouse up message is received from WebProcess

Period at the end :)

> Source/WebKit2/UIProcess/WebPageProxy.cpp:2146
> +NativeWebMouseEvent* WebPageProxy::currentMouseDownEvent()

Maybe this should be called "currentlyProcessedMouseDownEvent" or something along those lines, to indicate that this is the event that's been sent to the web process.

> Source/WebKit2/UIProcess/mac/WebPopupMenuProxyMac.mm:131
> +        // 57904: This code is adopted from EventHandler::sendFakeEventsAfterWidgetTracking()

Bug URL.

> Source/WebKit2/UIProcess/mac/WebPopupMenuProxyMac.mm:135
> +            NSEvent *fakeEvent = nil;

No need to initialize fakeEvent to nil. You can just initialize it with the created event.

> Source/WebKit2/UIProcess/mac/WebPopupMenuProxyMac.mm:148
> +                                           location:[[m_client->currentMouseDownEvent()->nativeView() window] convertScreenToBase:[NSEvent mouseLocation]]

Do we really need to store the native view in the event? Can't we just get the window from the WKView?

> Source/WebKit2/UIProcess/win/WebPopupMenuProxyWin.cpp:334
> +        const MSG* initiatingWinEvent = m_client->currentMouseDownEvent()->nativeEvent();
> +        MSG fakeEvent = *initiatingWinEvent;
> +        fakeEvent.message = WM_LBUTTONUP;
> +        ::DispatchMessage(&fakeEvent);

This could use a comment. Also, is DispatchMessage the right event here or should we use ::PostMessage instead?

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