Bug 90563 - [chromium] Add a method didChangeFormState to WebViewClient.
Summary: [chromium] Add a method didChangeFormState to WebViewClient.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Oli Lan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-04 09:08 PDT by Oli Lan
Modified: 2012-07-05 07:24 PDT (History)
7 users (show)

See Also:


Attachments
Patch (4.82 KB, patch)
2012-07-04 09:12 PDT, Oli Lan
no flags Details | Formatted Diff | Diff
Patch (7.57 KB, patch)
2012-07-05 06:42 PDT, Oli Lan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oli Lan 2012-07-04 09:08:17 PDT
[chromium] Add a method didChangeFocusedFormNodeState to WebViewClient.
Comment 1 Oli Lan 2012-07-04 09:12:12 PDT
Created attachment 150811 [details]
Patch
Comment 2 WebKit Review Bot 2012-07-04 09:15:20 PDT
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.
Comment 3 Adam Barth 2012-07-04 09:23:43 PDT
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() ?
Comment 4 Oli Lan 2012-07-04 09:56:29 PDT
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.
Comment 5 Adam Barth 2012-07-04 11:20:01 PDT
Yeah, let's do that. It will probably save us from having to add another callback later.
Comment 6 Oli Lan 2012-07-05 06:42:21 PDT
Created attachment 150935 [details]
Patch
Comment 7 Oli Lan 2012-07-05 06:45:05 PDT
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.
Comment 8 Adam Barth 2012-07-05 07:08:44 PDT
Comment on attachment 150935 [details]
Patch

Thanks!
Comment 9 WebKit Review Bot 2012-07-05 07:24:02 PDT
Comment on attachment 150935 [details]
Patch

Clearing flags on attachment: 150935

Committed r121904: <http://trac.webkit.org/changeset/121904>
Comment 10 WebKit Review Bot 2012-07-05 07:24:08 PDT
All reviewed patches have been landed.  Closing bug.