[webkit-reviews] review denied: [Bug 42922] [chromium] WebViewClient should have an elementClicked() method : [Attachment 62525] Really fixing style

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied Jay Civelli
<jcivelli at chromium.org>'s request for review:
Bug 42922: [chromium] WebViewClient should have an elementClicked() method
https://bugs.webkit.org/show_bug.cgi?id=42922

Attachment 62525: Really fixing style
https://bugs.webkit.org/attachment.cgi?id=62525&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
WebKit/chromium/src/WebViewImpl.cpp:386
 +	if (!clickedElement.isNull()) {
are you sure you want to run this callback right in the middle of
the mouseDown handler?	could that result in some interesting
re-entrancy bugs?  suppose the inputElementClicked method did
something to change focus.  could that be bad?	i see the code
following this checking focusedWebCoreNode.

WebKit/chromium/public/WebViewClient.h:155
 +	virtual void inputElementClicked(const WebInputElement& element, bool
alreadyFocused) { }
style-nit: no need for the "element" parameter name since it does not add
extra info.



WebKit/chromium/public/WebViewClient.h:155
 +	virtual void inputElementClicked(const WebInputElement& element, bool
alreadyFocused) { }
this name is a bit confusing.  based on the implementation, it seems that
this will happen whenever a mousedown event happens to result in the
focused node being an <input> element.	it does not necessarily mean
that the mousedown event was dispatched to an <input> element.	couldn't
i write an event handler that focused an <input> element during event
processing?  that would screw up this implementation, right?

if you really want to know if a "click" event was dispatched to an
<input> element, then I think you need to somehow hook into the
defaultEventHandler for the HTMLInputElement.  also, there is a
difference between the "click" event and the "mousedown" event.
i can't tell if that distinction matters to you.