WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
90563
[chromium] Add a method didChangeFormState to WebViewClient.
https://bugs.webkit.org/show_bug.cgi?id=90563
Summary
[chromium] Add a method didChangeFormState to WebViewClient.
Oli Lan
Reported
2012-07-04 09:08:17 PDT
[chromium] Add a method didChangeFocusedFormNodeState to WebViewClient.
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
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Oli Lan
Comment 1
2012-07-04 09:12:12 PDT
Created
attachment 150811
[details]
Patch
WebKit Review Bot
Comment 2
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
.
Adam Barth
Comment 3
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() ?
Oli Lan
Comment 4
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.
Adam Barth
Comment 5
2012-07-04 11:20:01 PDT
Yeah, let's do that. It will probably save us from having to add another callback later.
Oli Lan
Comment 6
2012-07-05 06:42:21 PDT
Created
attachment 150935
[details]
Patch
Oli Lan
Comment 7
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.
Adam Barth
Comment 8
2012-07-05 07:08:44 PDT
Comment on
attachment 150935
[details]
Patch Thanks!
WebKit Review Bot
Comment 9
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
>
WebKit Review Bot
Comment 10
2012-07-05 07:24:08 PDT
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.
Top of Page
Format For Printing
XML
Clone This Bug