[Webkit-unassigned] [Bug 60563] [Chromium]Click event is not fired for a menulist <select>

[bugzilla-daemon at webkit.org]

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





--- Comment #41 from Naoki Takano <takano.naoki at gmail.com>  2011-05-26 10:07:50 PST ---
Dimitri and Jay,

Thank you for review.

About test, I'll try it. But I've heard a couple of times, we don't have any good way to handle autofill test.

(In reply to comment #40)
> (From update of attachment 94898 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=94898&action=review
> 
> > Source/WebCore/platform/chromium/PopupMenuChromium.cpp:193
> > +    void acceptIndex(int index, const PlatformMouseEvent* event = 0);
> 
> this should just be const PlatformMouseEvent&.

But acceptIndex() is called by other functions where we don't have event. That's why I declared where with default value.

> > Source/WebCore/platform/chromium/PopupMenuChromium.cpp:283
> > +    Node* m_focusedNode;
> 
> This node could be removed from the tree while the popup is still open. I think you need a RefPtr here.

Ok


> > Source/WebCore/platform/chromium/PopupMenuChromium.cpp:1104
> > +            m_focusedNode->dispatchMouseEvent(*event, eventNames().mouseupEvent);
> 
> What's the re-entrance opportunity here? If up event removes the corresponding select element, are we still safe to continue this execution?
In this case, it should be fine if we use RefPtr.

But to clarify it, do we have to move this dispatch call to upper function PopupListBox::handleMouseReleaseEvent() from here?

If so, acceptIndex() don't have to take event anymore either. But acceptIndex() have to return boolean to check if we should forward event or not.

Thanks,

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