New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 410785 link

Starred by 19 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Feature

Blocked on:
issue 733197
issue 733218
issue 733222
issue 733282
issue 737009
issue 737388



Sign in to add a comment

Feature Request: on text entry have IME be smarter about go/next

Project Member Reported by benm@chromium.org, Sep 4 2014

Issue description

When filling out a multi-field form, the Android IME will show a "go" button in place of the enter key which makes it hard to navigate between fields in the form. Ideally, we'd use "next" in place of "go" until you get to the final editable field in the form.

(see also b/17367611 and https://code.google.com/p/android-developer-preview/issues/detail?id=1055
 
Currently for input type with number, or telephone number we are providing this option.
We have to extend this by checking whether there is any other extra input field in the page and inform IME to show NEXT instead of default GO. If only single input field is there, we can avoid showing NEXT.
To make this logic more effective, we need to get the next focusable input element from the page and try to focus it, rather than passing TAB_KEY event to blink.
Labels: M-40
Owner: bcwh...@chromium.org
Status: Assigned
Brian, can you or someone from your team look into this?


@klobag and bcwhite, if you have a plan to give this external contributor, I am happy to take this forward.
ajith.v@, can you first write a design doc? 

e.g. I don't think the logic to show "Next" is just as simple as checking whether there is another input field. A page can have two unrelated input fields. In that case, we don't want to show "Next".

Here are some concerns Aurimas listed in b/17367611

I spent 30 minutes searching for the bug, but I cannot seem to find it. The gist of the discussion was:
- You can either have next [1] or go [2] in the place of an enter key and not both.
- Setting button to be "next" could break websites that try to capture enter keys (we only send enter for go, but not for next) using javascript.
- There could be hidden/not visible fields that would break the logic of having "next" until you reach the last field and do "go" for the last field.
@klobag - Thanks for the details. I will prepare a design document after analyzing the code. Could you please share the format of design document, if chromium demands a particular format.

Agree with you, if the page has unrelated input field, then we don't have to provide option of NEXT there. If the input fields are child same of form field, we need to show NEXT as per the design. Additionally if the focus reaches last field of the form, we have to provide PREV option as well to make it more meaningful.

Yes we shouldn't give NEXT and GO together.
For element which contains Key event listeners (Enter key event listeners), we should keep GO button as well (Please correct me if I am wrong)
Hidden/disabled/read-only elements we should skip while judging the focus-able input element in the form.

In the design doc, I will update more details. Thanks
There is no fixed format, see examples in http://www.chromium.org/developers/design-documents.

Comment 7 by benm@chromium.org, Oct 22 2014

Cc: ajit...@samsung.com
ajith.v@, are you still planning to look at this?

Comment 8 by ajit...@samsung.com, Oct 23 2014

@benm I am preparing design document for this feature, once it's completed, I will upload for discussion and review here. Thanks

Comment 9 by ajit...@samsung.com, Oct 30 2014

Cc: benm@chromium.org
@klobag & benm I have uploaded the design document here. PTAL and share your thoughts. Based on it, I will plan the coding. Please apologize for the delay, as I was involved in other assignments.
Would my importing that into a Drive document and then enabling public comments?
@bcwhite, Could you please clarify your opinion ?
That was supposed to read:  Would you mind importing that into a Drive document and then enabling public comments?

Sorry for the bad text.

I was unable to open the file you attached; all the characters were garbled.
@bcwhite earlier problem was due to DRM. I was not able to upload MS word document. Here I am attaching Library Office doc. Once I resolve DRM issue, I will upload formatted MS word document.

Smarter-GO-NEXT-in-IME.doc
4.2 KB Download
Please review it and let me know your opinion
I've created a Drive version of your document and will comment there.
https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSPNg/edit?usp=sharing
@bcwhite Could you please look at updated design document @ https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSPNg/edit?usp=sharing. I fixed the review comments. Thank you.
Labels: -M-40 M-41 MovedFrom-40
Moving all non essential bugs to the next Milestone.
Cc: bcwh...@chromium.org
Labels: -M-41 -MovedFrom-40 UI-Input-Text-IME
Owner: ajit...@samsung.com
Status: Started
Cc: tedc...@chromium.org
I have did a skeleton implementation of this requirement. Currently working on stabilizing on live pages and "Blocking enter key from showing on Keyboard in case of single line text field without event listeners". Will upload the patch set, once pending item is completed.
 Issue 445446  has been merged into this issue.
 Issue #445446  relates to this as the action go|next is not set based on whether there are actually more fields.

I note, though, that the Android keyboard also has an IME_ACTION_PREVIOUS option so as long as you're doing "next", perhaps you should do "previous" as well.
Yes. I am already in progress with PREVIOUS as well. Thank you :)
@bcwhite - Currently I am facing some issue like when the element has both PREV and NEXT, then when we OR with EditorInfo.IME_ACTION_NEXT and EditorInfo.IME_ACTION_PREVIOUS with outAttrs.imeOptions, then it displays "<-|" same as enter key button, which is not correct. But when I OR only EditorInfo.IME_ACTION_NEXT, it displays NEXT key correctly. For solving this can we customize the full keyboard for Chrome ? AFAIK, this issue happens Keyboard doesn't have enough place to position both NEXT and PREV key at same time. Also for removing Enter Key from the IME, any other method we can do by keeping the imeOptions in Manifest itself ? Could you please share your thoughts on this ?

Comment 24 Deleted

If forced into displaying only one, then it should be "next".  But any fixes there will be the responsibility of the keyboard.

What device, android version, and keyboard version are you using?  I'll bring it up with the Keyboard team.

Cc: kochi@chromium.org yosin@chromium.org
@bcwhite - Thanks for the info. So if the element has both PREV and NEXT, we display NEXT only. But I feel we need to make room for showing PREV as well. Also we should be able to control the display of "Enter Key" button as well. Currently we can only control the "GO" button.
I am using Android 5.0.1 on Samsung Galaxy Note4 with Samsung Keyboard.
If you want me to test the changes on different device, pls let me know.
Exactly what the keyboard shows and how it behaves is not under Chrome's control.  Indeed, we don't even know _which_ keyboard is in use.  The best we can do is provide hints to make the experience better, which is what we're doing here.

So go ahead and pass all the appropriate flags/hints to IME and if there is a usability issue, we'll discuss it with the Keyboard team to see how things can be improved on one side or the other.
Cc: yfried...@chromium.org
@bcwhite - Thanks for your suggestions. I have progressed the way you suggested in previous comment and uploaded patch for review. I saw few issues, which we need to discuss with Keyboard team to sort out to prevent bad experience to the user with unwanted "enter key".
Labels: -UI-Input-Text-IME Cr-UI-Input-Text-IME
 Issue 476367  has been merged into this issue.
Cc: tkent@chromium.org
@bcwhite - tkent pointed out a problem when we have a content like below;
<form action="...">
<input type="text" name="input1">
<input type="text" name="input2">
</form>

Where the element input1 has no key listener, (Until now according to our logic, we don't show enter key), but for submitting this form, we still requires "Enter key" needs to be shown. So ultimately we have to show Enter Key always irrespective NEXT/PREVIOUS button status. This is a slight deviation from the design we finalized earlier. I thought to bring it in front of others. In that case checking of KeyEvent listener at blink side is not having any importance.
May be we can put like below;

1) If the element needs implicit form submission (above example) then we need to show GO key instead of Enter Key.
2) If the element has key listener [or in parent hierarchy], then we need to show Enter Key.
Either way Enter/GO will be visible along with PREV and NEXT.

@tkent, bcwhite and others please share your thoughts on this.
I am uploading a refined patch set based on above thoughts. Let's continue our discussion in review page. Thank you :)

Comment 33 by cdr...@gmail.com, Jun 9 2015

The inconsistency between <input type=text> (shows [->] key, generates enter key) and <input type=number> or tel (shows [>] key and generates tab key) is very odd.

e.g. http://output.jsbin.com/kegapi/2/quiet

Our user gets used to the "green" button doing one thing, but for a number field the "green" button does something different. As a developer it took me quite some time to work out why our form wasn't working as expected on Android (our form had a single numeric field on it).

Workaround: detect keydown on Android and submit() the form if tab pressed (event.keyCode == 9) - see http://output.jsbin.com/kegapi/2/quiet and tick the "Tab submits on Android" checkbox then focus to the number input and press [>] button.
@cdrcat - Please check your page, once I finish this changes. It needs some support from Android platform side as well as commented earlier, which will be mailed to bcwhite soon after I finish the changes. Thanks!
Project Member

Comment 35 by bugdroid1@chromium.org, Aug 12 2015

The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=200398

------------------------------------------------------------------
r200398 | ajith.v@chromium.org | 2015-08-12T15:40:24.531700Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/web/WebViewImpl.h?r1=200398&r2=200397&pathrev=200398
   A http://src.chromium.org/viewvc/blink/trunk/Source/web/tests/data/advance_focus_in_form_with_key_event_listeners.html?r1=200398&r2=200397&pathrev=200398
   A http://src.chromium.org/viewvc/blink/trunk/Source/web/tests/data/advance_focus_in_form_with_disabled_and_readonly_elements.html?r1=200398&r2=200397&pathrev=200398
   M http://src.chromium.org/viewvc/blink/trunk/public/web/WebTextInputType.h?r1=200398&r2=200397&pathrev=200398
   M http://src.chromium.org/viewvc/blink/trunk/Source/web/WebViewImpl.cpp?r1=200398&r2=200397&pathrev=200398
   M http://src.chromium.org/viewvc/blink/trunk/Source/web/tests/WebViewTest.cpp?r1=200398&r2=200397&pathrev=200398
   A http://src.chromium.org/viewvc/blink/trunk/Source/web/tests/data/advance_focus_in_form_with_tabindex_elements.html?r1=200398&r2=200397&pathrev=200398
   M http://src.chromium.org/viewvc/blink/trunk/public/web/WebView.h?r1=200398&r2=200397&pathrev=200398

Adding support for Smart GO NEXT feature in Android Chrome

This change takes care of providing easy navigation among text input
elements inside a form.

Corresponding changes to control the PREVIOUS, NEXT and GO button is done
at https://codereview.chromium.org/1080693002/

BUG= 410785 

Review URL: https://codereview.chromium.org/939603002
-----------------------------------------------------------------
Project Member

Comment 36 by bugdroid1@chromium.org, Aug 21 2015

The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=200948

------------------------------------------------------------------
r200948 | tkent@chromium.org | 2015-08-21T01:53:00.488703Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/public/web/WebTextInputType.h?r1=200948&r2=200947&pathrev=200948
   M http://src.chromium.org/viewvc/blink/trunk/Source/web/WebViewImpl.cpp?r1=200948&r2=200947&pathrev=200948
   M http://src.chromium.org/viewvc/blink/trunk/Source/web/tests/WebViewTest.cpp?r1=200948&r2=200947&pathrev=200948
   D http://src.chromium.org/viewvc/blink/trunk/Source/web/tests/data/advance_focus_in_form_with_tabindex_elements.html?r1=200948&r2=200947&pathrev=200948
   M http://src.chromium.org/viewvc/blink/trunk/public/web/WebView.h?r1=200948&r2=200947&pathrev=200948
   M http://src.chromium.org/viewvc/blink/trunk/Source/web/WebViewImpl.h?r1=200948&r2=200947&pathrev=200948
   D http://src.chromium.org/viewvc/blink/trunk/Source/web/tests/data/advance_focus_in_form_with_key_event_listeners.html?r1=200948&r2=200947&pathrev=200948
   D http://src.chromium.org/viewvc/blink/trunk/Source/web/tests/data/advance_focus_in_form_with_disabled_and_readonly_elements.html?r1=200948&r2=200947&pathrev=200948

Revert of Adding support for Smart GO NEXT feature in Android Chrome (patchset #23 id:460001 of https://codereview.chromium.org/939603002/ )

Reason for revert:
Regression in power.android_acceptance.
 crbug.com/520952 


Original issue's description:
> Adding support for Smart GO NEXT feature in Android Chrome
> 
> This change takes care of providing easy navigation among text input
> elements inside a form.
> 
> Corresponding changes to control the PREVIOUS, NEXT and GO button is done
> at https://codereview.chromium.org/1080693002/
> 
> BUG= 410785 
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=200398

TBR=kochi@chromium.org,l.gombos@samsung.com,ajith.v@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 410785 , 520952 

Review URL: https://codereview.chromium.org/1303123002
-----------------------------------------------------------------
Project Member

Comment 37 by bugdroid1@chromium.org, Sep 23 2015

Labels: merge-merged-2490
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/da0e5c32745ecfbceb66f2e6fadf552e30e66d0b

commit da0e5c32745ecfbceb66f2e6fadf552e30e66d0b
Author: ajith.v@chromium.org <ajith.v@chromium.org>
Date: Wed Aug 12 15:40:24 2015

Adding support for Smart GO NEXT feature in Android Chrome

This change takes care of providing easy navigation among text input
elements inside a form.

Corresponding changes to control the PREVIOUS, NEXT and GO button is done
at https://codereview.chromium.org/1080693002/

BUG= 410785 

Review URL: https://codereview.chromium.org/939603002

git-svn-id: svn://svn.chromium.org/blink/trunk@200398 bbb929c8-8fbe-4397-9dbb-9b2b20218538

[modify] http://crrev.com/da0e5c32745ecfbceb66f2e6fadf552e30e66d0b/third_party/WebKit/Source/web/WebViewImpl.cpp
[modify] http://crrev.com/da0e5c32745ecfbceb66f2e6fadf552e30e66d0b/third_party/WebKit/Source/web/WebViewImpl.h
[modify] http://crrev.com/da0e5c32745ecfbceb66f2e6fadf552e30e66d0b/third_party/WebKit/Source/web/tests/WebViewTest.cpp
[add] http://crrev.com/da0e5c32745ecfbceb66f2e6fadf552e30e66d0b/third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_disabled_and_readonly_elements.html
[add] http://crrev.com/da0e5c32745ecfbceb66f2e6fadf552e30e66d0b/third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_key_event_listeners.html
[add] http://crrev.com/da0e5c32745ecfbceb66f2e6fadf552e30e66d0b/third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_tabindex_elements.html
[modify] http://crrev.com/da0e5c32745ecfbceb66f2e6fadf552e30e66d0b/third_party/WebKit/public/web/WebTextInputType.h
[modify] http://crrev.com/da0e5c32745ecfbceb66f2e6fadf552e30e66d0b/third_party/WebKit/public/web/WebView.h

Project Member

Comment 38 by bugdroid1@chromium.org, Sep 23 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5312f6f029e562f0b341ae5ad282d6244b697b0f

commit 5312f6f029e562f0b341ae5ad282d6244b697b0f
Author: tkent@chromium.org <tkent@chromium.org>
Date: Fri Aug 21 01:53:00 2015

Revert of Adding support for Smart GO NEXT feature in Android Chrome (patchset #23 id:460001 of https://codereview.chromium.org/939603002/ )

Reason for revert:
Regression in power.android_acceptance.
 crbug.com/520952 


Original issue's description:
> Adding support for Smart GO NEXT feature in Android Chrome
> 
> This change takes care of providing easy navigation among text input
> elements inside a form.
> 
> Corresponding changes to control the PREVIOUS, NEXT and GO button is done
> at https://codereview.chromium.org/1080693002/
> 
> BUG= 410785 
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=200398

TBR=kochi@chromium.org,l.gombos@samsung.com,ajith.v@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 410785 , 520952 

Review URL: https://codereview.chromium.org/1303123002

git-svn-id: svn://svn.chromium.org/blink/trunk@200948 bbb929c8-8fbe-4397-9dbb-9b2b20218538

[modify] http://crrev.com/5312f6f029e562f0b341ae5ad282d6244b697b0f/third_party/WebKit/Source/web/WebViewImpl.cpp
[modify] http://crrev.com/5312f6f029e562f0b341ae5ad282d6244b697b0f/third_party/WebKit/Source/web/WebViewImpl.h
[modify] http://crrev.com/5312f6f029e562f0b341ae5ad282d6244b697b0f/third_party/WebKit/Source/web/tests/WebViewTest.cpp
[delete] http://crrev.com/5b066328bbf2ef318c8e51e5be0f0153f84855a1/third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_disabled_and_readonly_elements.html
[delete] http://crrev.com/5b066328bbf2ef318c8e51e5be0f0153f84855a1/third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_key_event_listeners.html
[delete] http://crrev.com/5b066328bbf2ef318c8e51e5be0f0153f84855a1/third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_tabindex_elements.html
[modify] http://crrev.com/5312f6f029e562f0b341ae5ad282d6244b697b0f/third_party/WebKit/public/web/WebTextInputType.h
[modify] http://crrev.com/5312f6f029e562f0b341ae5ad282d6244b697b0f/third_party/WebKit/public/web/WebView.h

Labels: Hotlist-Recharge
This issue likely requires triage.  The current issue owner maybe inactive (i.e. hasn't fixed an issue in the last 30 days).  Thanks for helping out!

-Anthony
Project Member

Comment 40 by bugdroid1@chromium.org, Sep 24 2015

He's active.  He needs help from the Android Keyboard team.  There's an open issue about it.  b/22198735
Just to update you all, we have requested Android Keyboard team to give required support to control the display of some keys (Enter, Go, etc.). It was not yet completed from their side. So not able to progress on this feature further.
As bcwhite indicated in #41, b/22198735 is opened to track that issue from Android Keyboard team. Thank you!
Cc: -aurimas@chromium.org
Project Member

Comment 44 by sheriffbot@chromium.org, Jul 10 2016

Labels: Hotlist-OpenBugWithCL
A change has landed for this issue, but it's been open for over 6 months. Please review and close it if applicable. If this issue should remain open, remove the "Hotlist-OpenBugWithCL" label. If no action is taken, it will be archived in 30 days.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 45 by torne@chromium.org, Jul 12 2016

Labels: -Hotlist-OpenBugWithCL
Still in progress.
Need Android Keyboard team's support for continuing this feature. Earlier bcwhite were following up with Android Keyboard team. As per earlier update from bcwhite, keyboard team is not taken that issue with high priority. Hence this feature request is still open :(
Cc: yabinh@chromium.org changwan@chromium.org aelias@chromium.org
 Issue 648986  has been merged into this issue.
I don't think this should wait for Google keyboard UX change as this is an inherent bug in Chrome app / WebView. And it seems that NEXT and GO icons are distinguishable already. You can try any number input and we show 'NEXT' as a pure guess now: https://jsfiddle.net/rkcvzjos/

ajith.v@, do you plan to continue what you were working on?

I think we should also check whether or not there is a submittable form. If there isn't any, we should set IME_ACTION_NONE instead.

@changwan - Thanks for your suggestions.
When I tested the changes in Samsung devices and Nexus 7 GED, I couldn't get both NEXT and PREV together until L platform. Due to which my changes will result (in case I wanted to show NEXT and PREV together) display issues in IME while tapping on an input field, which has PREV and NEXT elements.

Note: I haven't checked the issue in N OS yet. I will check and resume my work if found there is no dependency from IME side. Otheriwse I will report the issues here, which may be blocking my work.
Hmm... I didn't think that PREV is your focus of interest here.

The current logic of deciding on IME action is a pure guess: it sometimes adds EditorInfo.IME_ACTION_NEXT and it sometimes adds IME_ACTION_GO solely based on the input type not based on a real requirement of the website.

 Issue 648986  is a real bug because we have assigned IME_ACTION_NEXT incorrectly and IME sends 'tab' key event. We ended up moving focus to the URL bar.

For that issue, I had a similar-minded CL: https://codereview.chromium.org/2392463002/

but kochi@ kindly pointed me to your CL. I think the better option for us would be to 1) fix all the issues mentioned above 2) as well as fix PREV case 3) wait for Android / Google Keyboard to implement the enhanced UX.

If you're not interested in 1), I can take over that part as it seems to be a serious issue by itself.

@changwan - Thank you for informing about your patch CL.
If I omit the case of PREV in my patch @ https://codereview.chromium.org/1080693002/ & https://codereview.chromium.org/939603002. That can solve this problem as well.
If you are fine with it, I can apply refined patch from above CL by pointing to  issue 648986 . But I will wait for your consent before resuming my work.
Super! That sounds good to me. BTW, kochi@ and I had a brief chat and are wondering if you could calculate the next available element only once per focus change for optimization. (I think that might have to do with why your original CL got reverted.)
@changwan - Thank you for the confirmation. I will check the problem of optimization as you suggested and refine the patch ASAP.
Project Member

Comment 54 by bugdroid1@chromium.org, Jun 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8d80ad1410dcb23f55c323a97e57eed750a611dc

commit 8d80ad1410dcb23f55c323a97e57eed750a611dc
Author: ajith.v <ajith.v@samsung.com>
Date: Tue Jun 13 20:21:13 2017

[Android] Adding Smart GO/NEXT feature in Chrome

Smart Go/Next brings better user experience to the user during form submitting applications.
For navigating between form elements, user can use NEXT/PREVIOUS button from IME
without touching on individual fields. This will avoid unnecessary form submissions before
filling or visiting all fields in the form.

Additionally it will save user time and avoid redundant network requests before actually
filling/attending entire fields in the form

Design Document: https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSPNg/edit?usp=sharing

BUG= 410785 , 648986 

Review-Url: https://codereview.chromium.org/2839993002
Cr-Commit-Position: refs/heads/master@{#479126}

[modify] https://crrev.com/8d80ad1410dcb23f55c323a97e57eed750a611dc/content/browser/android/ime_adapter_android.cc
[modify] https://crrev.com/8d80ad1410dcb23f55c323a97e57eed750a611dc/content/browser/android/ime_adapter_android.h
[modify] https://crrev.com/8d80ad1410dcb23f55c323a97e57eed750a611dc/content/common/frame_messages.h
[modify] https://crrev.com/8d80ad1410dcb23f55c323a97e57eed750a611dc/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[modify] https://crrev.com/8d80ad1410dcb23f55c323a97e57eed750a611dc/content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java
[modify] https://crrev.com/8d80ad1410dcb23f55c323a97e57eed750a611dc/content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java
[modify] https://crrev.com/8d80ad1410dcb23f55c323a97e57eed750a611dc/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java
[modify] https://crrev.com/8d80ad1410dcb23f55c323a97e57eed750a611dc/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestInputMethodManagerWrapper.java
[modify] https://crrev.com/8d80ad1410dcb23f55c323a97e57eed750a611dc/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/8d80ad1410dcb23f55c323a97e57eed750a611dc/content/renderer/render_frame_impl.h
[modify] https://crrev.com/8d80ad1410dcb23f55c323a97e57eed750a611dc/content/test/data/android/input/input_forms.html
[modify] https://crrev.com/8d80ad1410dcb23f55c323a97e57eed750a611dc/third_party/WebKit/Source/core/editing/InputMethodController.cpp
[modify] https://crrev.com/8d80ad1410dcb23f55c323a97e57eed750a611dc/third_party/WebKit/Source/core/page/FocusController.cpp
[modify] https://crrev.com/8d80ad1410dcb23f55c323a97e57eed750a611dc/third_party/WebKit/Source/core/page/FocusController.h
[modify] https://crrev.com/8d80ad1410dcb23f55c323a97e57eed750a611dc/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/8d80ad1410dcb23f55c323a97e57eed750a611dc/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[modify] https://crrev.com/8d80ad1410dcb23f55c323a97e57eed750a611dc/third_party/WebKit/Source/web/tests/WebViewTest.cpp
[add] https://crrev.com/8d80ad1410dcb23f55c323a97e57eed750a611dc/third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_disabled_and_readonly_elements.html
[add] https://crrev.com/8d80ad1410dcb23f55c323a97e57eed750a611dc/third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_key_event_listeners.html
[add] https://crrev.com/8d80ad1410dcb23f55c323a97e57eed750a611dc/third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_tabindex_elements.html
[modify] https://crrev.com/8d80ad1410dcb23f55c323a97e57eed750a611dc/third_party/WebKit/public/BUILD.gn
[modify] https://crrev.com/8d80ad1410dcb23f55c323a97e57eed750a611dc/third_party/WebKit/public/platform/WebFocusType.h
[modify] https://crrev.com/8d80ad1410dcb23f55c323a97e57eed750a611dc/third_party/WebKit/public/platform/WebTextInputType.h
[modify] https://crrev.com/8d80ad1410dcb23f55c323a97e57eed750a611dc/third_party/WebKit/public/web/WebLocalFrame.h

Status: Fixed (was: Started)
Project Member

Comment 56 by bugdroid1@chromium.org, Jun 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/76fc114441c30b033be4b3aa74dba49eba5ad072

commit 76fc114441c30b033be4b3aa74dba49eba5ad072
Author: kochi <kochi@chromium.org>
Date: Thu Jun 15 04:25:23 2017

Revert of [Android] Adding Smart GO/NEXT feature in Chrome (patchset #26 id:500001 of https://codereview.chromium.org/2839993002/ )

Reason for revert:
This caused several clusterfuzz bugs
(733197, 733218, 733282) - let me revert for
investigation on those.

Original issue's description:
> [Android] Adding Smart GO/NEXT feature in Chrome
>
> Smart Go/Next brings better user experience to the user during form submitting applications.
> For navigating between form elements, user can use NEXT/PREVIOUS button from IME
> without touching on individual fields. This will avoid unnecessary form submissions before
> filling or visiting all fields in the form.
>
> Additionally it will save user time and avoid redundant network requests before actually
> filling/attending entire fields in the form
>
> Design Document: https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSPNg/edit?usp=sharing
>
> BUG= 410785 , 648986 
>
> Review-Url: https://codereview.chromium.org/2839993002
> Cr-Commit-Position: refs/heads/master@{#479126}
> Committed: https://chromium.googlesource.com/chromium/src/+/8d80ad1410dcb23f55c323a97e57eed750a611dc

TBR=tkent@chromium.org,changwan@chromium.org,dcheng@chromium.org,ekaramad@chromium.org,nasko@chromium.org,yosin@chromium.org,aelias@chromium.org,ajith.v@samsung.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 410785 , 648986 

Review-Url: https://codereview.chromium.org/2938123002
Cr-Commit-Position: refs/heads/master@{#479601}

[modify] https://crrev.com/76fc114441c30b033be4b3aa74dba49eba5ad072/content/browser/android/ime_adapter_android.cc
[modify] https://crrev.com/76fc114441c30b033be4b3aa74dba49eba5ad072/content/browser/android/ime_adapter_android.h
[modify] https://crrev.com/76fc114441c30b033be4b3aa74dba49eba5ad072/content/common/frame_messages.h
[modify] https://crrev.com/76fc114441c30b033be4b3aa74dba49eba5ad072/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[modify] https://crrev.com/76fc114441c30b033be4b3aa74dba49eba5ad072/content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java
[modify] https://crrev.com/76fc114441c30b033be4b3aa74dba49eba5ad072/content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java
[modify] https://crrev.com/76fc114441c30b033be4b3aa74dba49eba5ad072/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java
[modify] https://crrev.com/76fc114441c30b033be4b3aa74dba49eba5ad072/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestInputMethodManagerWrapper.java
[modify] https://crrev.com/76fc114441c30b033be4b3aa74dba49eba5ad072/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/76fc114441c30b033be4b3aa74dba49eba5ad072/content/renderer/render_frame_impl.h
[modify] https://crrev.com/76fc114441c30b033be4b3aa74dba49eba5ad072/content/test/data/android/input/input_forms.html
[modify] https://crrev.com/76fc114441c30b033be4b3aa74dba49eba5ad072/third_party/WebKit/Source/core/editing/InputMethodController.cpp
[modify] https://crrev.com/76fc114441c30b033be4b3aa74dba49eba5ad072/third_party/WebKit/Source/core/page/FocusController.cpp
[modify] https://crrev.com/76fc114441c30b033be4b3aa74dba49eba5ad072/third_party/WebKit/Source/core/page/FocusController.h
[modify] https://crrev.com/76fc114441c30b033be4b3aa74dba49eba5ad072/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/76fc114441c30b033be4b3aa74dba49eba5ad072/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[modify] https://crrev.com/76fc114441c30b033be4b3aa74dba49eba5ad072/third_party/WebKit/Source/web/tests/WebViewTest.cpp
[delete] https://crrev.com/05cb777e577c0a2b47f6b0ce552a1198d54e6478/third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_disabled_and_readonly_elements.html
[delete] https://crrev.com/05cb777e577c0a2b47f6b0ce552a1198d54e6478/third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_key_event_listeners.html
[delete] https://crrev.com/05cb777e577c0a2b47f6b0ce552a1198d54e6478/third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_tabindex_elements.html
[modify] https://crrev.com/76fc114441c30b033be4b3aa74dba49eba5ad072/third_party/WebKit/public/BUILD.gn
[modify] https://crrev.com/76fc114441c30b033be4b3aa74dba49eba5ad072/third_party/WebKit/public/platform/WebFocusType.h
[modify] https://crrev.com/76fc114441c30b033be4b3aa74dba49eba5ad072/third_party/WebKit/public/platform/WebTextInputType.h
[modify] https://crrev.com/76fc114441c30b033be4b3aa74dba49eba5ad072/third_party/WebKit/public/web/WebLocalFrame.h

Comment 57 by kochi@chromium.org, Jun 15 2017

Blockedon: 733197 733218 733282
Status: Assigned (was: Fixed)
Reopening.

Comment 58 by kochi@chromium.org, Jun 15 2017

Maybe those dependent bugs will be closed because the CL was reverted,
but please make sure these cases are addressed in the next CL.

Comment 59 by kochi@chromium.org, Jun 16 2017

Blockedon: 733222
Status: Started (was: Assigned)
Project Member

Comment 61 by bugdroid1@chromium.org, Jun 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4eb6812cc312e4ea919518a4e3087944e31b3b72

commit 4eb6812cc312e4ea919518a4e3087944e31b3b72
Author: ajith.v <ajith.v@samsung.com>
Date: Fri Jun 23 13:38:16 2017

Relanding [Android] Adding Smart GO/NEXT feature in Chrome

Smart Go/Next brings better user experience to the user during form submitting applications.
For navigating between form elements, user can use NEXT/PREVIOUS button from IME
without touching on individual fields. This will avoid unnecessary form submissions before
filling or visiting all fields in the form.

Additionally it will save user time and avoid redundant network requests before actually
filling/attending entire fields in the form

Design Document: https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSPNg/edit?usp=sharing

Initial patch is reviewed @ https://codereview.chromium.org/2839993002/

BUG= 410785 , 648986 ,  733222 

Review-Url: https://codereview.chromium.org/2948593002
Cr-Commit-Position: refs/heads/master@{#481868}

[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/content/browser/android/ime_adapter_android.cc
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/content/browser/android/ime_adapter_android.h
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/content/common/frame_messages.h
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestInputMethodManagerWrapper.java
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/content/renderer/render_frame_impl.h
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/content/test/data/android/input/input_forms.html
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/Source/core/editing/InputMethodController.cpp
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/Source/core/exported/WebViewTest.cpp
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/Source/core/page/FocusController.cpp
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/Source/core/page/FocusController.h
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/Source/core/page/FocusControllerTest.cpp
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[add] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_disabled_and_readonly_elements.html
[add] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_key_event_listeners.html
[add] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_tabindex_elements.html
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/public/BUILD.gn
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/public/platform/WebFocusType.h
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/public/platform/WebTextInputType.h
[modify] https://crrev.com/4eb6812cc312e4ea919518a4e3087944e31b3b72/third_party/WebKit/public/web/WebLocalFrame.h

Status: Fixed (was: Started)

Comment 63 by kochi@chromium.org, Jun 28 2017

Blockedon: 737388
Status: Assigned (was: Fixed)
This was reverted due to perf regression:  issue 737388 
Project Member

Comment 64 by bugdroid1@chromium.org, Jun 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fa75e2abb23205caa6d7f61edc89e672e83f6089

commit fa75e2abb23205caa6d7f61edc89e672e83f6089
Author: Takayoshi Kochi <kochi@chromium.org>
Date: Wed Jun 28 08:22:13 2017

Revert of Relanding [Android] Adding Smart GO/NEXT feature in Chrome (patchset #4 id:80001 of https://codereview.chromium.org/2948593002/ )

Reason for revert:
This caused perf regression on browse_social_facebook_infinite_scroll
test on most platforms.

See  crbug.com/737388  for details.

Original issue's description:
> Relanding [Android] Adding Smart GO/NEXT feature in Chrome
>
> Smart Go/Next brings better user experience to the user during form submitting applications.
> For navigating between form elements, user can use NEXT/PREVIOUS button from IME
> without touching on individual fields. This will avoid unnecessary form submissions before
> filling or visiting all fields in the form.
>
> Additionally it will save user time and avoid redundant network requests before actually
> filling/attending entire fields in the form
>
> Design Document: https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSPNg/edit?usp=sharing
>
> Initial patch is reviewed @ https://codereview.chromium.org/2839993002/
>
> BUG= 410785 , 648986 ,  733222 
>
> Review-Url: https://codereview.chromium.org/2948593002
> Cr-Commit-Position: refs/heads/master@{#481868}
> Committed: https://chromium.googlesource.com/chromium/src/+/4eb6812cc312e4ea919518a4e3087944e31b3b72

TBR=yosin@chromium.org,aelias@chromium.org,changwan@chromium.org,dcheng@chromium.org,kochi@chromium.org,tedchoc@chromium.org,tkent@chromium.org,yfriedman@chromium.org,nasko@chromium.org,ajith.v@samsung.com
BUG= 410785 , 648986 ,  733222 

patch from issue 2955283002 at patchset 1 (http://crrev.com/2955283002#ps1)

Change-Id: Idd861244a71d079581f7a1a6337c1ebad245ca71
Reviewed-on: https://chromium-review.googlesource.com/551697
Reviewed-by: Takayoshi Kochi <kochi@chromium.org>
Commit-Queue: Takayoshi Kochi <kochi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#482918}
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/content/browser/android/ime_adapter_android.cc
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/content/browser/android/ime_adapter_android.h
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/content/common/frame_messages.h
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestInputMethodManagerWrapper.java
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/content/renderer/render_frame_impl.h
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/content/test/data/android/input/input_forms.html
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/third_party/WebKit/Source/core/editing/InputMethodController.cpp
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/third_party/WebKit/Source/core/exported/WebViewTest.cpp
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/third_party/WebKit/Source/core/page/FocusController.cpp
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/third_party/WebKit/Source/core/page/FocusController.h
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/third_party/WebKit/Source/core/page/FocusControllerTest.cpp
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[delete] https://crrev.com/b919d95a7f8be9ca291d6bbff942cfb42d08d5a1/third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_disabled_and_readonly_elements.html
[delete] https://crrev.com/b919d95a7f8be9ca291d6bbff942cfb42d08d5a1/third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_key_event_listeners.html
[delete] https://crrev.com/b919d95a7f8be9ca291d6bbff942cfb42d08d5a1/third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_tabindex_elements.html
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/third_party/WebKit/public/BUILD.gn
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/third_party/WebKit/public/platform/WebFocusType.h
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/third_party/WebKit/public/platform/WebTextInputType.h
[modify] https://crrev.com/fa75e2abb23205caa6d7f61edc89e672e83f6089/third_party/WebKit/public/web/WebLocalFrame.h

Comment 65 by kochi@chromium.org, Jun 29 2017

Blockedon: 737009
Project Member

Comment 66 by bugdroid1@chromium.org, Jul 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/933aca75233fbb5145c29c1e5a06e5f36c308cc0

commit 933aca75233fbb5145c29c1e5a06e5f36c308cc0
Author: ajith.v <ajith.v@samsung.com>
Date: Mon Jul 03 17:05:26 2017

Relanding [Android] Adding Smart GO/NEXT feature in Chrome

Smart Go/Next brings better user experience to the user during form submitting applications.
For navigating between form elements, user can use NEXT/PREVIOUS button from IME
without touching on individual fields. This will avoid unnecessary form submissions before
filling or visiting all fields in the form.

Additionally it will save user time and avoid redundant network requests before actually
filling/attending entire fields in the form

Design Document: https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSPNg/edit?usp=sharing

Initial patch is reviewed @ https://codereview.chromium.org/2839993002/ and
https://codereview.chromium.org/2948593002/

Scrolling performance is getting effected due to call to
NextFocusableElementInForm() from InputMethodController. hence splitting it into multiple patches.
This is the initial landing without enabling functionality.
In next patch functionality will be enabled with necessary test coverage.

BUG= 410785 ,  648986 ,  733222 

Review-Url: https://codereview.chromium.org/2967493002
Cr-Commit-Position: refs/heads/master@{#484020}

[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/content/browser/android/ime_adapter_android.cc
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/content/browser/android/ime_adapter_android.h
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/content/common/frame_messages.h
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/content/renderer/render_frame_impl.h
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/third_party/WebKit/Source/core/page/FocusController.cpp
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/third_party/WebKit/Source/core/page/FocusController.h
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/third_party/WebKit/Source/core/page/FocusControllerTest.cpp
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/third_party/WebKit/public/BUILD.gn
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/third_party/WebKit/public/platform/WebFocusType.h
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/third_party/WebKit/public/platform/WebTextInputType.h
[modify] https://crrev.com/933aca75233fbb5145c29c1e5a06e5f36c308cc0/third_party/WebKit/public/web/WebLocalFrame.h

kochi@ and changwan@ - I will wait for few days to make sure this patch is stable enough before applying next change.

Meanwhile could you please let me know, how many days it will take to run that performance benchmark in release build ?

If you have any suggestion please let me know.
I don't think you have to wait - if some regression happens, the bot will
automatically identify which build got regressed. Unless 2 patches submitted
consecutively and got merged in one build, it will be fine.

Have you identified what/where exactly was causing the performance regression?
kochi@ - My first catch is fromhttps://codereview.chromium.org/2948593002/diff/80001/third_party/WebKit/Source/core/editing/InputMethodController.cpp-> InputMethodController::TextInputFlags() we are calling NextFocusableElementInForm(), which is an expensive operation. So introducing performance bottle neck.

But I am yet to identify how we can gracefully fix this problem.
Other operations prior to this patch is comparatively light weight operation. Hence no problem with performance.
Status: Started (was: Assigned)
Cc: -tkent@chromium.org
Project Member

Comment 74 by bugdroid1@chromium.org, Aug 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/041c0b075176fb9bca94fd67fc1b6053a4df2e1c

commit 041c0b075176fb9bca94fd67fc1b6053a4df2e1c
Author: AJITH KUMAR V <ajith.v@samsung.com>
Date: Tue Aug 08 10:39:20 2017

[Android] Relanding Smart GO NEXT feature in Android Chrome 2/2

This is second patch of Smart GO NEXT feature. Initial patch is
landed @ https://codereview.chromium.org/2967493002/

Design Document: https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSPNg/edit?usp=sharing

Performance regression is getting tackled using the triggering of Focus Controller
call only if element focus is changed, otherwise continue to use previously cached value.
This will save unwanted tree traversal in every frame update.

BUG= 410785 ,  648986 ,  733222 ,  737388 

Change-Id: Ib2c7343a6ec7dea18c7cfa5ac283ac4d29e3a4cb
Reviewed-on: https://chromium-review.googlesource.com/574514
Commit-Queue: AJITH KUMAR V <ajith.v@samsung.com>
Reviewed-by: Changwan Ryu <changwan@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Takayoshi Kochi <kochi@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492588}
[modify] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java
[modify] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java
[modify] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestInputMethodManagerWrapper.java
[modify] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/content/renderer/render_widget.cc
[modify] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/content/renderer/render_widget.h
[modify] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/content/test/data/android/input/input_forms.html
[modify] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/third_party/WebKit/Source/core/editing/InputMethodController.cpp
[modify] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/third_party/WebKit/Source/core/editing/InputMethodController.h
[modify] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/third_party/WebKit/Source/core/exported/WebInputMethodControllerImpl.cpp
[modify] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/third_party/WebKit/Source/core/exported/WebInputMethodControllerImpl.h
[modify] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/third_party/WebKit/Source/core/exported/WebViewTest.cpp
[add] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/third_party/WebKit/Source/core/testing/data/advance_focus_in_form_with_disabled_and_readonly_elements.html
[add] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/third_party/WebKit/Source/core/testing/data/advance_focus_in_form_with_key_event_listeners.html
[add] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/third_party/WebKit/Source/core/testing/data/advance_focus_in_form_with_tabindex_elements.html
[modify] https://crrev.com/041c0b075176fb9bca94fd67fc1b6053a4df2e1c/third_party/WebKit/public/web/WebInputMethodController.h

Labels: -Type-Bug Type-Feature
kochi@ and changwan@ - Now the patch is landed, how do we ensure this change goes for a thorough testing ? (Release notes or feature announcement we need to pass for the QA team ?)

Comment 76 by kochi@chromium.org, Aug 10 2017

Usually you just sit and wait for automated regression tests reports anything.
If nothing regresses, nothing happens :)

This implementation enables using the existing feature of Android IMEs, so
strictly spaking, no new UI addition is done in Android Chrome - so I don't
think you need any UI review or something.

For extra testing for this feature by QA team, I'll contact them.

I hope Google can include this in a blog post of M62 release, I'll contact
someone responsible for it and will cc you.

If anything that we can help, please let me know.
Thanks kochi@ for the detailed update. Let's wait for performance bot's signals :)
Project Member

Comment 78 by bugdroid1@chromium.org, Aug 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/97a2cec0f42fd791815d981c0fa462311771031c

commit 97a2cec0f42fd791815d981c0fa462311771031c
Author: AJITH KUMAR V <ajith.v@samsung.com>
Date: Fri Aug 11 19:58:34 2017

Remove least significant variable imeAction from ImeUtils

Instead of using a separate variable, apply the values directly
to outAttrs.imeOptions

Bug:  410785 
Change-Id: I17049301ba0fd02db507185bfdaadf6b48fde9f5
Reviewed-on: https://chromium-review.googlesource.com/611688
Reviewed-by: Changwan Ryu <changwan@chromium.org>
Commit-Queue: AJITH KUMAR V <ajith.v@samsung.com>
Cr-Commit-Position: refs/heads/master@{#493836}
[modify] https://crrev.com/97a2cec0f42fd791815d981c0fa462311771031c/content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java

Status: Fixed (was: Started)
This feature is enabled in Chromium now. Hence resolving master bug from here. Thank you all people for your detailed discussions and code reviews for landing this in Chromium.

Comment 80 by vbgu...@gmail.com, Nov 16 2017

Which version of Chrome has this bug fixed?

Comment 81 by kochi@chromium.org, Nov 16 2017

Re comment#80 Version 62, which is now stable version.

Comment 82 by gzavo...@gmail.com, Mar 23 2018

Can we add a similar issue, so that this works on input fields that are outside of forms as well? 
I think there is not much benefit with it. From a form, taking user to somewhere entirely different area is not really worth. So IMO, current implementation is good enough.

Comment 84 by gzavo...@gmail.com, Mar 26 2018

From design point of view, we have many implementations where we have different input fields between sections.. 

By the way, before Chrome was used for webview; Android Webview did this fine. 
I don't have a strong opinion on this! Happy to hear from others about the importance of this extension.

Comment 86 by gzavo...@gmail.com, Mar 26 2018

Specifically, we have several customers that have input prompts inside of seperate <div> tags; would be a nice help to several accounts. 
kochi@ and yosin@ might be better people to answer this, but I think it would be a good idea to extend this logic to formless input cases to be consistent with the old tab behavior.

But would the current performance optimization logic remain valid in those cases? We may need to tread carefully and optimize it further for the new cases.

Sign in to add a comment