Changed Blimp IME to use PopUp

Changed Blimp IME to use PopUp

Blimp IME currently shows an EditText on the top of the soft keyboard.
This CL changes it to use a popup instead based on an AlertDialog.
WebInputBox was renamed to ImeHelperDialog which contains an ImeEditText.
This change also fixes the issue of view resize due to IME.

BUG=611111
Committed: https://crrev.com/10a8e63afa2ca6e91671293fa6c2781c32ea1162
Cr-Commit-Position: refs/heads/master@{#415819}

[shaktisahu, 2016-08-19 22:36:54]

shaktisahu@chromium.org changed reviewers:
+ dtrainor@chromium.org, nyquist@chromium.org

[shaktisahu, 2016-08-19 22:36:55]

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
File
blimp/client/app/android/java/src/org/chromium/blimp/input/ImeHelperDialog.java
(right):

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/input/ImeHelperDialog.java:91:
.setTitle("Enter Text")
This will be changed to reflect the actual field name (e.g. Email or Phone
Number)

[David Trainor- moved to gerrit, 2016-08-22 21:25:25]

looking good!

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
File blimp/client/app/android/java/res/layout/text_input_popup.xml (right):

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/res/layout/text_input_popup.xml:9:
android:id="@+id/edit_text"
Should we make this id a bit more specific?

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
File
blimp/client/app/android/java/src/org/chromium/blimp/BlimpRendererActivity.java
(right):

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/BlimpRendererActivity.java:56:
private ImeHelperDialog mImeHelperDialog;
We'll have to make this driven by native at some point so we can properly set up
the feature and delegate interactions without having to go through Java.

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/BlimpRendererActivity.java:170:
mImeHelperDialog = new ImeHelperDialog(this);
We won't have access to the Activity Context in 0.6.  Is that going to cause
issues?

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
File blimp/client/app/android/java/src/org/chromium/blimp/input/ImeEditText.java
(right):

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/input/ImeEditText.java:14:
* A {@link EditText} that allows user to enter text into a web page.
This description seems a bit too specific, given that this class just is an
EditText that closes a parent dialog when KEYCODE_BACK is pressed.  Should we
make it more generic?

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/input/ImeEditText.java:38:
mParentDialog.dismiss();
What's the current behavior with the back button?

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
File
blimp/client/app/android/java/src/org/chromium/blimp/input/ImeHelperDialog.java
(right):

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/input/ImeHelperDialog.java:34:
private Context mContext;
final?

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/input/ImeHelperDialog.java:37:
public ImeHelperDialog(Context context) {
Javadoc

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/input/ImeHelperDialog.java:68:
public void onImeTextEntered(String text) {
Does this have to be public?

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/input/ImeHelperDialog.java:91:
.setTitle("Enter Text")
On 2016/08/19 22:36:55, shaktisahu wrote:
> This will be changed to reflect the actual field name (e.g. Email or Phone
> Number)

Put the string inside the proper grd file.

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/input/ImeHelperDialog.java:140:
* @param inputTypeEngine Input type received from engine, defined in {@link
TextInputType}.
Add the editText param

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/input/ImeHelperDialog.java:142:
private void setEditorOptions(EditText editText, int inputTypeEngine) {
static?

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
File blimp/client/app/android/java/strings/android_blimp_strings.grd (right):

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/strings/android_blimp_strings.grd:184: <message
name="IDS_OK" desc="Label for a confirm button. Used in multiple contexts.
[CHAR-LIMIT=20]">
IDS_WEB_FORM_INPUT_OK?

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/strings/android_blimp_strings.grd:187: <message
name="IDS_CANCEL" desc="Label for a cancel button. Used in multiple contexts.
[CHAR-LIMIT=20]">
IDS_WEB_FORM_INPUT_CANCEL?

[shaktisahu, 2016-08-23 17:35:15]

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
File blimp/client/app/android/java/res/layout/text_input_popup.xml (right):

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/res/layout/text_input_popup.xml:9:
android:id="@+id/edit_text"
On 2016/08/22 21:25:24, David Trainor wrote:
> Should we make this id a bit more specific?

Done.

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
File
blimp/client/app/android/java/src/org/chromium/blimp/BlimpRendererActivity.java
(right):

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/BlimpRendererActivity.java:56:
private ImeHelperDialog mImeHelperDialog;
On 2016/08/22 21:25:24, David Trainor wrote:
> We'll have to make this driven by native at some point so we can properly set
up
> the feature and delegate interactions without having to go through Java.

Acknowledged. I will work towards it in a subsequent CL.

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/BlimpRendererActivity.java:170:
mImeHelperDialog = new ImeHelperDialog(this);
On 2016/08/22 21:25:24, David Trainor wrote:
> We won't have access to the Activity Context in 0.6.  Is that going to cause
> issues?

What type of context will we have?
LayoutInflater and AlertDialog need the context.

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
File blimp/client/app/android/java/src/org/chromium/blimp/input/ImeEditText.java
(right):

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/input/ImeEditText.java:14:
* A {@link EditText} that allows user to enter text into a web page.
On 2016/08/22 21:25:24, David Trainor wrote:
> This description seems a bit too specific, given that this class just is an
> EditText that closes a parent dialog when KEYCODE_BACK is pressed.  Should we
> make it more generic?

:). Changed the description a bit. I think the class name itself is specific for
this purpose.

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/input/ImeEditText.java:38:
mParentDialog.dismiss();
On 2016/08/22 21:25:24, David Trainor wrote:
> What's the current behavior with the back button?

Currently, we hide the IME and make the EditText invisible. Also we go back to
"adjustNothing" option. Check WebInputBox.dispatchKeyEventPreIme.

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
File
blimp/client/app/android/java/src/org/chromium/blimp/input/ImeHelperDialog.java
(right):

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/input/ImeHelperDialog.java:34:
private Context mContext;
On 2016/08/22 21:25:25, David Trainor wrote:
> final?

Done.

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/input/ImeHelperDialog.java:37:
public ImeHelperDialog(Context context) {
On 2016/08/22 21:25:24, David Trainor wrote:
> Javadoc

Done.

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/input/ImeHelperDialog.java:68:
public void onImeTextEntered(String text) {
On 2016/08/22 21:25:25, David Trainor wrote:
> Does this have to be public?

Done.

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/input/ImeHelperDialog.java:91:
.setTitle("Enter Text")
On 2016/08/22 21:25:25, David Trainor wrote:
> On 2016/08/19 22:36:55, shaktisahu wrote:
> > This will be changed to reflect the actual field name (e.g. Email or Phone
> > Number)
> 
> Put the string inside the proper grd file.

Done.

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/input/ImeHelperDialog.java:140:
* @param inputTypeEngine Input type received from engine, defined in {@link
TextInputType}.
On 2016/08/22 21:25:24, David Trainor wrote:
> Add the editText param

Done.

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/input/ImeHelperDialog.java:142:
private void setEditorOptions(EditText editText, int inputTypeEngine) {
On 2016/08/22 21:25:24, David Trainor wrote:
> static?

Done.

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
File blimp/client/app/android/java/strings/android_blimp_strings.grd (right):

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/strings/android_blimp_strings.grd:184: <message
name="IDS_OK" desc="Label for a confirm button. Used in multiple contexts.
[CHAR-LIMIT=20]">
On 2016/08/22 21:25:25, David Trainor wrote:
> IDS_WEB_FORM_INPUT_OK?

OK and Cancel are available in Chrome .grd file already with names IDS_OK and
IDS_CANCEL. I didn't want to duplicate that when moving to Chrome. So I had to
copy the same name to the blimp .grd file for the blimp app.

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/strings/android_blimp_strings.grd:187: <message
name="IDS_CANCEL" desc="Label for a cancel button. Used in multiple contexts.
[CHAR-LIMIT=20]">
On 2016/08/22 21:25:25, David Trainor wrote:
> IDS_WEB_FORM_INPUT_CANCEL?

See above.

[David Trainor- moved to gerrit, 2016-08-29 04:28:50]

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
File blimp/client/app/android/java/src/org/chromium/blimp/input/ImeEditText.java
(right):

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/input/ImeEditText.java:38:
mParentDialog.dismiss();
On 2016/08/23 17:35:14, shaktisahu wrote:
> On 2016/08/22 21:25:24, David Trainor wrote:
> > What's the current behavior with the back button?
> 
> Currently, we hide the IME and make the EditText invisible. Also we go back to
> "adjustNothing" option. Check WebInputBox.dispatchKeyEventPreIme.

I guess what I'm asking is does back on the phone currently dismiss the dialog? 
What does it do without this code?

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
File blimp/client/app/android/java/strings/android_blimp_strings.grd (right):

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/strings/android_blimp_strings.grd:184: <message
name="IDS_OK" desc="Label for a confirm button. Used in multiple contexts.
[CHAR-LIMIT=20]">
On 2016/08/23 17:35:14, shaktisahu wrote:
> On 2016/08/22 21:25:25, David Trainor wrote:
> > IDS_WEB_FORM_INPUT_OK?
> 
> OK and Cancel are available in Chrome .grd file already with names IDS_OK and
> IDS_CANCEL. I didn't want to duplicate that when moving to Chrome. So I had to
> copy the same name to the blimp .grd file for the blimp app.

This code probably will never live in chrome/.  It'll stay in blimp/.  IIUC this
means we can't use the chrome strings, right?

[shaktisahu, 2016-08-29 18:58:47]

David, PTAL

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
File blimp/client/app/android/java/src/org/chromium/blimp/input/ImeEditText.java
(right):

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/src/org/chromium/blimp/input/ImeEditText.java:38:
mParentDialog.dismiss();
On 2016/08/29 04:28:50, David Trainor wrote:
> On 2016/08/23 17:35:14, shaktisahu wrote:
> > On 2016/08/22 21:25:24, David Trainor wrote:
> > > What's the current behavior with the back button?
> > 
> > Currently, we hide the IME and make the EditText invisible. Also we go back
to
> > "adjustNothing" option. Check WebInputBox.dispatchKeyEventPreIme.
> 
> I guess what I'm asking is does back on the phone currently dismiss the
dialog? 
> What does it do without this code?

Oh.. Without this code, the phone takes two back presses to dismiss the dialog,
first one to close the IME and second one to dismiss the dialog. With this, it
will dismiss the IME and the dialog simultaneously.

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
File blimp/client/app/android/java/strings/android_blimp_strings.grd (right):

https://codereview.chromium.org/2261853002/diff/1/blimp/client/app/android/ja...
blimp/client/app/android/java/strings/android_blimp_strings.grd:184: <message
name="IDS_OK" desc="Label for a confirm button. Used in multiple contexts.
[CHAR-LIMIT=20]">
On 2016/08/29 04:28:50, David Trainor wrote:
> On 2016/08/23 17:35:14, shaktisahu wrote:
> > On 2016/08/22 21:25:25, David Trainor wrote:
> > > IDS_WEB_FORM_INPUT_OK?
> > 
> > OK and Cancel are available in Chrome .grd file already with names IDS_OK
and
> > IDS_CANCEL. I didn't want to duplicate that when moving to Chrome. So I had
to
> > copy the same name to the blimp .grd file for the blimp app.
> 
> This code probably will never live in chrome/.  It'll stay in blimp/.  IIUC
this
> means we can't use the chrome strings, right?

Ok, makes sense. Thanks for making this clear. I will change this string now.

[David Trainor- moved to gerrit, 2016-08-30 19:28:40]

lgtm!

[shaktisahu, 2016-08-30 20:23:40]

The CQ bit was checked by shaktisahu@chromium.org

[commit-bot: I haz the power, 2016-08-30 20:24:50]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-30 20:35:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-30 20:35:17]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[shaktisahu, 2016-08-31 00:16:15]

shaktisahu@chromium.org changed reviewers:
+ mikecase@chromium.org

[shaktisahu, 2016-08-31 00:16:16]

@mikecase, PTAL at the build/android/lint/suppressions.xml

[nyquist, 2016-08-31 05:33:39]

lgtm

[mikecase (-- gone --), 2016-08-31 20:36:52]

build/android/lint/suppressions.xml lgtm

[shaktisahu, 2016-08-31 21:58:14]

The CQ bit was checked by shaktisahu@chromium.org

[shaktisahu, 2016-08-31 21:58:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org
Link to the patchset: https://codereview.chromium.org/2261853002/#ps100001
(title: "Fixed xml attributes and lint warnings")

[commit-bot: I haz the power, 2016-08-31 21:59:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-31 22:03:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-31 22:03:09]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[shaktisahu, 2016-08-31 22:41:31]

The CQ bit was checked by shaktisahu@chromium.org

[shaktisahu, 2016-08-31 22:41:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
nyquist@chromium.org, dtrainor@chromium.org, mikecase@chromium.org
Link to the patchset: https://codereview.chromium.org/2261853002/#ps120001
(title: "merge origin/master")

[commit-bot: I haz the power, 2016-08-31 22:42:26]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-31 22:50:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-31 22:50:57]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[nyquist, 2016-08-31 23:23:45]

https://codereview.chromium.org/2261853002/diff/120001/blimp/client/BUILD.gn
File blimp/client/BUILD.gn (right):

https://codereview.chromium.org/2261853002/diff/120001/blimp/client/BUILD.gn#...
blimp/client/BUILD.gn:247: deps = [
I think you want to add
//third_party/android_tools:android_support_v7_appcompat_java

[shaktisahu, 2016-08-31 23:34:35]

The CQ bit was checked by shaktisahu@chromium.org

[shaktisahu, 2016-08-31 23:34:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
nyquist@chromium.org, dtrainor@chromium.org, mikecase@chromium.org
Link to the patchset: https://codereview.chromium.org/2261853002/#ps140001
(title: "Used AlertDialog from support.v7")

[commit-bot: I haz the power, 2016-08-31 23:35:00]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-01 00:29:00]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-09-01 00:30:54]

Description was changed from

==========
Changed Blimp IME to use PopUp

Blimp IME currently shows an EditText on the top of the soft keyboard.
This CL changes it to use a popup instead based on an AlertDialog.
WebInputBox was renamed to ImeHelperDialog which contains an ImeEditText.
This change also fixes the issue of view resize due to IME.

BUG=611111
==========

to

==========
Changed Blimp IME to use PopUp

Blimp IME currently shows an EditText on the top of the soft keyboard.
This CL changes it to use a popup instead based on an AlertDialog.
WebInputBox was renamed to ImeHelperDialog which contains an ImeEditText.
This change also fixes the issue of view resize due to IME.

BUG=611111

Committed: https://crrev.com/10a8e63afa2ca6e91671293fa6c2781c32ea1162
Cr-Commit-Position: refs/heads/master@{#415819}
==========

[commit-bot: I haz the power, 2016-09-01 00:30:56]

Patchset 8 (id:??) landed as
https://crrev.com/10a8e63afa2ca6e91671293fa6c2781c32ea1162
Cr-Commit-Position: refs/heads/master@{#415819}