New issue
Advanced search Search tips

Issue 830959 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 0
Type: Bug

Blocking:
issue 821570



Sign in to add a comment

[CRD iOS] Tapping return doesn't inject Enter to Windows host

Project Member Reported by yuweih@chromium.org, Apr 9 2018

Issue description

App Version: 67.0.3390.0
OS Version: 11.3

What steps will reproduce the problem?
(1) Connect to a Windows host
(2) Open Chrome
(3) Type a URL and press `return`

What is the expected result?
Chrome goes to the URL.

What happens instead?
Nothing happens.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 10 2018

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

commit 3298a607d7e8b39d8426591a1edc48b8cf7eaa7a
Author: Yuwei Huang <yuweih@chromium.org>
Date: Tue Apr 10 02:05:00 2018

[CRD iOS] Set keyboard type to ASCII capable

Currently our keyboard type is default, which allows complex input
methods like Chinese IME to be selected, but we haven't implemented
support for it yet so they will never work. This CL simply limits the
keyboard type to ASCII capable.

Bug:  830959 
Change-Id: I24b05975d2e242059d4fa94c7e42444ecf7e5bc4
Reviewed-on: https://chromium-review.googlesource.com/1003774
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549377}
[modify] https://crrev.com/3298a607d7e8b39d8426591a1edc48b8cf7eaa7a/remoting/ios/client_keyboard.mm

Comment 2 by yuweih@chromium.org, Apr 10 2018

Summary: [CRD iOS] Tapping return doesn't inject Enter to Windows host (was: [CRD iOS] Enter key doesn't work for all apps)
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 10 2018

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

commit 8b79a3c6208a35ca2667be9f2f2dd94b64198d1b
Author: Yuwei Huang <yuweih@chromium.org>
Date: Tue Apr 10 02:29:29 2018

[CRD iOS] Translate "\n" into an enter keypress

iOS is sending a "\n" unicode text event to the host when the user taps
the `return` key. On Windows this doesn't always generate a newline.

This CL fixes this issue by translating the "\n" text event into a
ENTER keypress before sending it to the host.

Bug:  830959 
Change-Id: Ib94caf6ce2036cc5654f1b59f1e0136c81c9eec1
Reviewed-on: https://chromium-review.googlesource.com/1003598
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549385}
[modify] https://crrev.com/8b79a3c6208a35ca2667be9f2f2dd94b64198d1b/remoting/client/input/keyboard_interpreter.cc
[modify] https://crrev.com/8b79a3c6208a35ca2667be9f2f2dd94b64198d1b/remoting/client/input/keyboard_interpreter.h
[modify] https://crrev.com/8b79a3c6208a35ca2667be9f2f2dd94b64198d1b/remoting/ios/app/host_view_controller.mm
[modify] https://crrev.com/8b79a3c6208a35ca2667be9f2f2dd94b64198d1b/remoting/ios/client_keyboard.h
[modify] https://crrev.com/8b79a3c6208a35ca2667be9f2f2dd94b64198d1b/remoting/ios/client_keyboard.mm

Comment 4 by yuweih@chromium.org, Apr 10 2018

Labels: Merge-Request-66
We need to merge this into M66. Currently Chrome Remote Desktop users cannot press the Enter key on a Windows host.

Risk:
These changes only affect Chromoting for iOS and doesn't affect Chrome.

Test:
Tested on ToT build of Chrome Remote Desktop (67.0.3394.0) and verified that I can inject the Enter key on the Windows host.
Project Member

Comment 5 by sheriffbot@chromium.org, Apr 10 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: Less than 3 days to go before AppStore submit on M66
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

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

Comment 6 by yuweih@chromium.org, Apr 10 2018

Labels: -Hotlist-Merge-Review -Merge-Review-66
Let me hold off this merge request. It looks like user will not be able to type non-ASCII character at all if they had turned on IME. We may potentially need more radical changes in the way we handle key inputs.
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 10 2018

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

commit d9da0824074316204959952ce960901c3fe11310
Author: Yuwei Huang <yuweih@chromium.org>
Date: Tue Apr 10 20:34:28 2018

Revert "[CRD iOS] Translate "\n" into an enter keypress"

This reverts commit 8b79a3c6208a35ca2667be9f2f2dd94b64198d1b.

Reason for revert: This change is not good enough to fix the keypress issue we see. We need to rework the way we send keypress, which will undo the change we do here.

Original change's description:
> [CRD iOS] Translate "\n" into an enter keypress
> 
> iOS is sending a "\n" unicode text event to the host when the user taps
> the `return` key. On Windows this doesn't always generate a newline.
> 
> This CL fixes this issue by translating the "\n" text event into a
> ENTER keypress before sending it to the host.
> 
> Bug:  830959 
> Change-Id: Ib94caf6ce2036cc5654f1b59f1e0136c81c9eec1
> Reviewed-on: https://chromium-review.googlesource.com/1003598
> Commit-Queue: Yuwei Huang <yuweih@chromium.org>
> Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#549385}

TBR=jamiewalch@chromium.org,yuweih@chromium.org

Change-Id: Icba4c28dccc7154f30a41b90dc2cbc62feae2824
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  830959 
Reviewed-on: https://chromium-review.googlesource.com/1005697
Reviewed-by: Yuwei Huang <yuweih@chromium.org>
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549633}
[modify] https://crrev.com/d9da0824074316204959952ce960901c3fe11310/remoting/client/input/keyboard_interpreter.cc
[modify] https://crrev.com/d9da0824074316204959952ce960901c3fe11310/remoting/client/input/keyboard_interpreter.h
[modify] https://crrev.com/d9da0824074316204959952ce960901c3fe11310/remoting/ios/app/host_view_controller.mm
[modify] https://crrev.com/d9da0824074316204959952ce960901c3fe11310/remoting/ios/client_keyboard.h
[modify] https://crrev.com/d9da0824074316204959952ce960901c3fe11310/remoting/ios/client_keyboard.mm

Comment 8 by yuweih@chromium.org, Apr 10 2018

Labels: Merge-Request-66
We need to get comment#1 and this CL https://chromium-review.googlesource.com/c/chromium/src/+/1006065 merged into M66. Comment#3 has been reverted and doesn't need to be merged.

I'm sending the merge request early as the beta bot will soon be run. If we miss the build then we need to wait for one more day.

Note that this only affects Chrome Remote Desktop but not Chrome.
Project Member

Comment 9 by sheriffbot@chromium.org, Apr 10 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: Less than 3 days to go before AppStore submit on M66
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Review -Merge-Review-66 Merge-Approved-66
Project Member

Comment 11 by bugdroid1@chromium.org, Apr 11 2018

Labels: -merge-approved-66 merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f84ffa46bb7cd5f9d89a874ba9d87d1c137f4503

commit f84ffa46bb7cd5f9d89a874ba9d87d1c137f4503
Author: Yuwei Huang <yuweih@chromium.org>
Date: Wed Apr 11 00:35:22 2018

[CRD iOS] Set keyboard type to ASCII capable

Currently our keyboard type is default, which allows complex input
methods like Chinese IME to be selected, but we haven't implemented
support for it yet so they will never work. This CL simply limits the
keyboard type to ASCII capable.

Bug:  830959 
Change-Id: I24b05975d2e242059d4fa94c7e42444ecf7e5bc4
Reviewed-on: https://chromium-review.googlesource.com/1003774
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#549377}(cherry picked from commit 3298a607d7e8b39d8426591a1edc48b8cf7eaa7a)
Reviewed-on: https://chromium-review.googlesource.com/1005958
Reviewed-by: Yuwei Huang <yuweih@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#668}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/f84ffa46bb7cd5f9d89a874ba9d87d1c137f4503/remoting/ios/client_keyboard.mm

Project Member

Comment 12 by bugdroid1@chromium.org, Apr 11 2018

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

commit 34ade990750ea181d21d1ace52d804406d8302c7
Author: Yuwei Huang <yuweih@chromium.org>
Date: Wed Apr 11 01:06:20 2018

[CRD iOS] Implement Char-to-Keycode mapping logic for US keyboard layout

Previously we don't bother to implement a keycode mapping logic and
we directly send texts to the host to inject. Hosts usually handle text
injection very differently than key injection and may end up texts being
directly injected to the host without passing through IME. In other
words, host side IME doesn't work with our current key injection
implementation.

The downside of this change is that non-US keyboard user will now get
the wrong keyboard output on the host again, which is what we had on the
v1 client.

For longer term, we can add a new host setting for changing the keyboard
layout.

NOTRY=true

Bug:  830959 
Change-Id: Ia34b6894fd7c0a7a07a5b9657aca19c2a878bd4e
Reviewed-on: https://chromium-review.googlesource.com/1006065
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Gary Kacmarcik <garykac@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549706}
[modify] https://crrev.com/34ade990750ea181d21d1ace52d804406d8302c7/remoting/client/input/BUILD.gn
[modify] https://crrev.com/34ade990750ea181d21d1ace52d804406d8302c7/remoting/client/input/keyboard_interpreter.cc
[modify] https://crrev.com/34ade990750ea181d21d1ace52d804406d8302c7/remoting/client/input/keyboard_interpreter.h
[add] https://crrev.com/34ade990750ea181d21d1ace52d804406d8302c7/remoting/client/input/keycode_map.cc
[add] https://crrev.com/34ade990750ea181d21d1ace52d804406d8302c7/remoting/client/input/keycode_map.h
[add] https://crrev.com/34ade990750ea181d21d1ace52d804406d8302c7/remoting/client/input/keycode_map_unittest.cc
[modify] https://crrev.com/34ade990750ea181d21d1ace52d804406d8302c7/remoting/ios/app/host_view_controller.mm
[modify] https://crrev.com/34ade990750ea181d21d1ace52d804406d8302c7/remoting/ios/client_keyboard.h
[modify] https://crrev.com/34ade990750ea181d21d1ace52d804406d8302c7/remoting/ios/client_keyboard.mm

Project Member

Comment 13 by bugdroid1@chromium.org, Apr 11 2018

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

commit f946d76f6c4f7e9bbb412193ae96f702b4c6a4f7
Author: Yuwei Huang <yuweih@chromium.org>
Date: Wed Apr 11 01:28:17 2018

[CRD iOS] Implement Char-to-Keycode mapping logic for US keyboard layout

Previously we don't bother to implement a keycode mapping logic and
we directly send texts to the host to inject. Hosts usually handle text
injection very differently than key injection and may end up texts being
directly injected to the host without passing through IME. In other
words, host side IME doesn't work with our current key injection
implementation.

The downside of this change is that non-US keyboard user will now get
the wrong keyboard output on the host again, which is what we had on the
v1 client.

For longer term, we can add a new host setting for changing the keyboard
layout.

NOTRY=true
TBR=yuweih@chromium.org

Bug:  830959 
Change-Id: Ia34b6894fd7c0a7a07a5b9657aca19c2a878bd4e
Reviewed-on: https://chromium-review.googlesource.com/1006065
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Gary Kacmarcik <garykac@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#549706}
Reviewed-on: https://chromium-review.googlesource.com/1005967
Reviewed-by: Yuwei Huang <yuweih@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#671}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/f946d76f6c4f7e9bbb412193ae96f702b4c6a4f7/remoting/client/input/BUILD.gn
[modify] https://crrev.com/f946d76f6c4f7e9bbb412193ae96f702b4c6a4f7/remoting/client/input/keyboard_interpreter.cc
[modify] https://crrev.com/f946d76f6c4f7e9bbb412193ae96f702b4c6a4f7/remoting/client/input/keyboard_interpreter.h
[add] https://crrev.com/f946d76f6c4f7e9bbb412193ae96f702b4c6a4f7/remoting/client/input/keycode_map.cc
[add] https://crrev.com/f946d76f6c4f7e9bbb412193ae96f702b4c6a4f7/remoting/client/input/keycode_map.h
[add] https://crrev.com/f946d76f6c4f7e9bbb412193ae96f702b4c6a4f7/remoting/client/input/keycode_map_unittest.cc
[modify] https://crrev.com/f946d76f6c4f7e9bbb412193ae96f702b4c6a4f7/remoting/ios/app/host_view_controller.mm
[modify] https://crrev.com/f946d76f6c4f7e9bbb412193ae96f702b4c6a4f7/remoting/ios/client_keyboard.h
[modify] https://crrev.com/f946d76f6c4f7e9bbb412193ae96f702b4c6a4f7/remoting/ios/client_keyboard.mm

Well, we still missed the build :(
Status: Fixed (was: Assigned)
Blocking: 821570
Status: Verified (was: Fixed)
Verified Fixed in 67.0.3396.22

Sign in to add a comment