Bug 41286

Summary: [chromium] EditorClient should forward text field related calls to the WebViewClient
Product: WebKit Reporter: Jay Civelli <jcivelli>
Component: WebKit Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dpranke, fishd
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
First version of the patch
fishd: review-
Updated with fishd's suggested changes
none
Patch for landing none

Description Jay Civelli 2010-06-28 10:53:15 PDT
In order to be able to move password autocomplete and field autocomplete code from EditorClientImpl.cpp to the Chromium code, the text field related methods should be exposed on the WebViewClient.
Comment 1 Jay Civelli 2010-06-28 13:25:44 PDT
Created attachment 59930 [details]
First version of the patch
Comment 2 Dirk Pranke 2010-07-02 12:27:28 PDT
Comment on attachment 59930 [details]
First version of the patch

Looks basically good to me, but I'm neither an expert on this area of the code nor a reviewer :)
Comment 3 Darin Fisher (:fishd, Google) 2010-07-02 13:38:35 PDT
Comment on attachment 59930 [details]
First version of the patch

WebKit/chromium/src/EditorClientImpl.cpp:823
 +      HTMLInputElement* inputElement = WebKit::toHTMLInputElement(element);
EditorClientImpl is defined in the WebKit namespace, so you don't
need to have the WebKit:: prefix before toHTMLInputElement.

WebKit/chromium/public/WebViewClient.h:146
 +      virtual void textFieldHandlingKeyDown(const WebInputElement&, const WebKeyboardEvent&) { }
textFieldDidReceiveKeyDown?

WebKit/chromium/public/WebViewClient.h:144
 +      virtual void textDidChangeInTextField(const WebInputElement&) { }
textFieldDidChange?

WebKit/chromium/public/WebInputElement.h:83
 +          WEBKIT_API bool readOnly() const;
hmm... maybe this should be named isReadOnly to match isEnabledFormControl.
i know you are just replicating readOnly from the WebCore name.

looks good otherwise.
Comment 4 Jay Civelli 2010-07-02 15:11:53 PDT
Created attachment 60411 [details]
Updated with fishd's suggested changes
Comment 5 Darin Fisher (:fishd, Google) 2010-07-02 22:11:41 PDT
Comment on attachment 60411 [details]
Updated with fishd's suggested changes

WebKit/chromium/public/WebViewClient.h:142
 +      // Called when a key down happens in a text-field.
this comment seems fairly redundant now with the method name, so you could
leave it out.

R=me
Comment 6 Jay Civelli 2010-07-07 17:12:45 PDT
Created attachment 60808 [details]
Patch for landing
Comment 7 WebKit Commit Bot 2010-07-07 17:44:41 PDT
Comment on attachment 60808 [details]
Patch for landing

Rejecting patch 60808 from commit-queue.

jcivelli@chromium.org does not have committer permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in WebKitTools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  Due to bug 30084 the commit-queue will require a restart after your change.  Please contact eseidel@chromium.org to request a commit-queue restart.  After restart the commit-queue will correctly respect your committer rights.
Comment 8 WebKit Commit Bot 2010-07-08 22:26:05 PDT
Comment on attachment 60808 [details]
Patch for landing

Clearing flags on attachment: 60808

Committed r62893: <http://trac.webkit.org/changeset/62893>
Comment 9 WebKit Commit Bot 2010-07-08 22:26:10 PDT
All reviewed patches have been landed.  Closing bug.