In order to factor out the autofill client from WebKit to Chromium, Chromium needs an easy way of knowing an input element has been clicked and whether or not that element had focus before it was clicked. This is so autofill suggestion popups can be shown when an input element is clicked.
Created attachment 62480 [details] Initial patch
Attachment 62480 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit/chromium/public/WebViewClient.h:153: One or more unexpected \r (^M) found; better to use only a \n [whitespace/carriage_return] [1] Suppressing further [whitespace/carriage_return] reports for this file. WebKit/chromium/public/WebViewClient.h:153: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/chromium/public/WebViewClient.h:154: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/chromium/public/WebViewClient.h:155: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 5 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 62481 [details] Fixed style.
Attachment 62481 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit/chromium/public/WebViewClient.h:153: One or more unexpected \r (^M) found; better to use only a \n [whitespace/carriage_return] [1] Suppressing further [whitespace/carriage_return] reports for this file. Total errors found: 2 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
LGTM, minus style fixes.
Created attachment 62525 [details] Really fixing style
Comment on attachment 62525 [details] Really fixing style 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.
For reference, the "click" event is defined here: http://www.w3.org/TR/DOM-Level-2-Events/events.html#Events-MouseEvent Notice that a "click" corresponds to a mousedown followed by a mouseup. Have you considered just using an event listener?
@fishd Sorry my patch was wrong. I meant to have clickedElement be the clickedNode, not the focused element. As you suggest, I could probably use an event listener from the Chromium side instead. I could add an event listener for mousedown on the document in render_view.cc I have to use mousedown instead of click because by the time I get notified of the click, the mousedown event already focused the field and I have no way of knowing if the input field was focused before it was clicked (I need to know that so we can have the autofill behavior where clicking a focused input shows the autofill popup). I am not sure there is a reentrency problem in that case, but I can see how some other listeners might change the focus. In that case we probably would not want to notify for autofill purposes. I could post a task so to process the notification and check which element is focused after all listeners have returned (once the stack has unwinded). Do you think it makes sense?
Addressed on the Chromium side.