Bug 90563

Summary: [chromium] Add a method didChangeFormState to WebViewClient.
Product: WebKit Reporter: Oli Lan <olilan>
Component: New BugsAssignee: Oli Lan <olilan>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dglazkov, eric, fishd, jamesr, tkent+wkapi, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Oli Lan
Reported Wednesday, July 4, 2012 5:08:17 PM UTC
[chromium] Add a method didChangeFocusedFormNodeState to WebViewClient.
Attachments
Patch (4.82 KB, patch)
2012-07-04 09:12 PDT, Oli Lan
no flags
Patch (7.57 KB, patch)
2012-07-05 06:42 PDT, Oli Lan
no flags
Oli Lan
Comment 1 Wednesday, July 4, 2012 5:12:12 PM UTC
WebKit Review Bot
Comment 2 Wednesday, July 4, 2012 5:15:20 PM UTC
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Adam Barth
Comment 3 Wednesday, July 4, 2012 5:23:43 PM UTC
Comment on attachment 150811 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150811&action=review > Source/WebKit/chromium/src/ChromeClientImpl.cpp:845 > + if (node->focused() && m_webView->client()) > + m_webView->client()->didChangeFocusedFormNodeState(); Should we pass |node| to the client, or does it not care? Should we have a client callback that we call unconditionally and let the embedder test for node->focused() ?
Oli Lan
Comment 4 Wednesday, July 4, 2012 5:56:29 PM UTC
Comment on attachment 150811 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150811&action=review >> Source/WebKit/chromium/src/ChromeClientImpl.cpp:845 >> + m_webView->client()->didChangeFocusedFormNodeState(); > > Should we pass |node| to the client, or does it not care? Should we have a client callback that we call unconditionally and let the embedder test for node->focused() ? For the current purposes of the Android port this is sufficient, but passing the node unconditionally and allowing the client to perform whatever logic is required makes sense. Happy to make this change.
Adam Barth
Comment 5 Wednesday, July 4, 2012 7:20:01 PM UTC
Yeah, let's do that. It will probably save us from having to add another callback later.
Oli Lan
Comment 6 Thursday, July 5, 2012 2:42:21 PM UTC
Oli Lan
Comment 7 Thursday, July 5, 2012 2:45:05 PM UTC
OK, the new patch implements the above. As WebNode did not have a focused method, the patch also adds a focused method to WebNode to allow the new method to be used as discussed.
Adam Barth
Comment 8 Thursday, July 5, 2012 3:08:44 PM UTC
Comment on attachment 150935 [details] Patch Thanks!
WebKit Review Bot
Comment 9 Thursday, July 5, 2012 3:24:02 PM UTC
Comment on attachment 150935 [details] Patch Clearing flags on attachment: 150935 Committed r121904: <http://trac.webkit.org/changeset/121904>
WebKit Review Bot
Comment 10 Thursday, July 5, 2012 3:24:08 PM UTC
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.