Let WebViewImpl::textInputInfo() query the focused element when selection is empty

Let WebViewImpl::textInputInfo() query the focused element when selection is empty

This function should return the most accurate information about current text input
based on focus and selection. This change rearranges it so that it can return available
values based on querying the focused element even if the selection is not updated yet.
This will be used in chromium to fix an input method related issue.

BUG=318460
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=163685

[kbalazs, 2013-12-07 00:29:25]

This is the Blink pair of https://codereview.chromium.org/59553003/ to fix
http://crbug.com/318460

[kbalazs, 2013-12-07 00:31:38]

So what this patch does is just that it moves the queries based on focus before
the ones based on selection so when we early return because selection is empty
we still provide some info.

[kbalazs, 2013-12-07 00:46:53]

Do you know how to run webview tests on a try bot? (i.e. gtest unittests in
web/tests/WebViewTest.cpp)?

[aurimas (slooooooooow), 2013-12-07 00:49:01]

On 2013/12/07 00:46:53, kbalazs wrote:
> Do you know how to run webview tests on a try bot? (i.e. gtest unittests in
> web/tests/WebViewTest.cpp)?

I think its a part of linux_blink

[abarth-chromium, 2013-12-07 22:07:29]

> I think its a part of linux_blink

That's right.

Can you add a test for this change?  WebViewTest.cpp is likely the right file to
add the test to.  You can run them locally by building webkit_unit_tests.

[kbalazs, 2013-12-10 00:13:13]

On 2013/12/07 22:07:29, abarth wrote:
> > I think its a part of linux_blink
> 
> That's right.
> 
> Can you add a test for this change?  WebViewTest.cpp is likely the right file
to
> add the test to.  You can run them locally by building webkit_unit_tests.

I have problems with the unit test. The textInputInfo() does not give me the
expected result for my test page. The test page just have an input field and js
to focus it. Do you think that I can expect that the inline javascript in the
page will be executed at the time when m_webViewHelper.initializeAndLoad()
returns? If not than I don't understand how other tests with js can work since I
cannot find anything in the unit tests that forces javascript execution.

Other input related tests are using webView->setInitialFocus() which
automatically focuses the first focusable node. This is not an option for me as
it looks like it is using another code path than the one triggered by focus()
from javascript. In other words I cannot trigger the bug I am trying to fix if I
call setInitialFocus.

Here is my manual test that I used to verify the patch:
https://gist.github.com/balazs/7883280. It installs a touch handler that use
preventDefault, so the input field does not get focus as a result of default
behavior. Instead it calls focus() in the handler (when event.target is the
input field). So when I call webview->textInputInfo() from
ChromeClient::focusedNodeChanged(), it only gives the right answer with the
patch. Does that make you and idea about what should I do in the unit test?

[kbalazs, 2013-12-10 00:38:34]

It seems like javascript does not run at all for these tests. I checked
hello_world.html. I does document.write('Hello world!') but
webView->mainFrame->contentAsText() is empty when I call it from the unit test
function (if I add some static content that it contains that).

[kbalazs, 2013-12-10 01:39:37]

On 2013/12/10 00:38:34, kbalazs wrote:
> It seems like javascript does not run at all for these tests. I checked
> hello_world.html. I does document.write('Hello world!') but
> webView->mainFrame->contentAsText() is empty when I call it from the unit test
> function (if I add some static content that it contains that).

Which was probably not the best way to test it, so I rather tried to find the
dynamic content by traversing the node tree and still did not find it.

[kbalazs, 2013-12-11 01:03:17]

Unit test added.

[abarth-chromium, 2013-12-11 06:26:10]

lgtm

Thanks!

[commit-bot: I haz the power, 2013-12-11 06:26:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/b.kelemen@samsung.com/103863007/20001

[commit-bot: I haz the power, 2013-12-11 07:17:09]

Change committed as 163685

[adamk, 2013-12-11 22:50:13]

I think this caused a regression on Android in
contentshell_instrumentation_tests:

http://build.chromium.org/p/chromium.webkit/builders/Android%20Tests%20%28dbg...

[adamk, 2013-12-11 22:58:21]

On 2013/12/11 22:50:13, adamk wrote:
> I think this caused a regression on Android in
> contentshell_instrumentation_tests:
> 
>
http://build.chromium.org/p/chromium.webkit/builders/Android%2520Tests%2520%2...

Reverted in https://src.chromium.org/viewvc/blink?revision=163732&view=revision

[kbalazs, 2013-12-12 21:21:26]

I'm trying to run the test locally and I have this error:

I   32s 602d  {'class':
'org.chromium.content.browser.input.ImeTest#testImeSelectAndUnSelectAll'}
# 12/12/13 11:53:56: am instrument -e class
'org.chromium.content.browser.input.ImeTest#testImeSelectAndUnSelectAll' -r -w
org.chromium.content_shell_apk.tests/android.test.InstrumentationTestRunner
# 12/12/13 11:53:57: No code available. inst_finished_bundle contains: {} 
W   33s 602d  Will retry test, try #1.
I   33s 602d  [host]> adb devices
D   33s 602d  List of devices attached 
4d00d10e589d602d        device

I used this command: test_runner.py instrumentation --release -vvv
--test-apk=ContentShellTest -A Feature=TextInput (also the filter does not
work).

I followed instructions at
http://www.chromium.org/developers/testing/android-tests. Do you have any idea
what can be wrong?

[kbalazs, 2013-12-12 22:55:38]

Could you copy the diff between the actual and the expected output? The link is
dead (at least for me).

I'm not sure I will be able reproduce this locally. Anyway, one thing I can
think about to fix is to introduce a new api on WebView, say
textInputInfoForFocusedElement() and use that in the chromium patch. Do you
think it sounds good?

[kbalazs, 2013-12-19 19:29:39]

On 2013/12/12 22:55:38, kbalazs wrote:
> Could you copy the diff between the actual and the expected output? The link
is
> dead (at least for me).
> 
> I'm not sure I will be able reproduce this locally. Anyway, one thing I can
> think about to fix is to introduce a new api on WebView, say
> textInputInfoForFocusedElement() and use that in the chromium patch. Do you
> think it sounds good?

I think the good way to fix this is to change the expectation in the unit test.
I'm speculating as I don't have the test output. It does not seem to be easy to
run these tests locally (my dev phone is actually getting mad with the
instrumentation tests and shuts down). If you could copy the output from the
failure it would help (I cannot access the link).

Do you know which try-bot runs these tests, if any?

[kbalazs, 2014-01-24 21:16:43]

FYI it turned out that my solution was not good because we don't want to allow
js to trigger the virtual keyboard. So this is abandoned.