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

Issue 714771 link

Starred by 7 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Two backspaces required to delete last character in webview input

Reported by kevinsaw...@gmail.com, Apr 24 2017

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.81 Safari/537.36

Steps to reproduce the problem:
1. Install Chrome extension with <webview> tag with single input field, https://jsfiddle.net/5vyoL8pz/2/
2. Switch keyboard preference to Katakana
3. Press the w keyboard key
4. Press the backspace keyboard key

What is the expected behavior?
Character is deleted

What went wrong?
Character is not deleted, a second backspace press is required to delete it.

Did this work before? N/A 

Chrome version: 58.0.3029.81  Channel: stable
OS Version: OS X 10.12.4
Flash Version: 

Also tried in canary 60.0.3079.0 and saw the same behavior.
 
webview-backspace.gif
55.4 KB View Download
Components: -UI Platform>Extensions
Labels: Needs-Triage-M58
Labels: -Pri-2 Needs-Bisect Prestable-58.0.3029.81 Pri-1
Cc: krajshree@chromium.org
Labels: Needs-Feedback
kevinsawicki@ - Thanks for filing the issue...!!

Could you please provide a sample chrome extension(with <webview> tag with single input field)URL as on opening the URL: https://jsfiddle.net/5vyoL8pz/2/ in chrome leads to a blank output.
Hence, unable to proceed with the bug.

Thanks...!!
The full extension can be found in this repository, https://github.com/kevinsawicki/webview-backspace-extension
Project Member

Comment 6 by sheriffbot@chromium.org, Apr 25 2017

Labels: -Needs-Feedback
Thank you for providing more feedback. Adding requester "krajshree@chromium.org" to the cc list and removing "Needs-Feedback" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: ekaramad@chromium.org
Labels: -Type-Bug -Needs-Bisect -Needs-Triage-M58 M-60 hasbisect Type-Bug-Regression
Owner: nona@chromium.org
Status: Assigned (was: Unconfirmed)
Able to reproduce the issue on Mac 10.12.4 using chrome reported version #58.0.3029.81 and latest chrome version #60.0.3080.5.

Bisect Information:
=====================
Good build: 54.0.2824.0  Revision(410520)
Bad Build : 54.0.2826.0  Revision(411209)

Change Log URL: 
https://chromium.googlesource.com/chromium/src/+log/46b00b4b4ffd56cc47b9126216a432245d5f4483..b02023adefda06d9afa8ec55ff892221b073e7bd

From the above change log suspecting below change

Review url: https://codereview.chromium.org/2121953002

nona@ - Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Thanks...!!

Comment 8 by nona@chromium.org, Apr 26 2017

Cc: nona@chromium.org
Owner: yosin@chromium.org
I'm sorry I don't have enough bandwidth to investigate this issue.
yosin@, please triage this issue.

Comment 9 by yosin@chromium.org, Apr 27 2017

Labels: -M-60

Comment 10 by yosin@chromium.org, Apr 27 2017

Components: Blink>Editing
Labels: -Pri-1 Pri-2
Owner: ----
Status: Available (was: Assigned)
Lower to Pri-2, since this regression is not found 24 weeks (introduced on M54 and found on M58.)

Comment 11 by nya@chromium.org, May 19 2017

Cc: nya@chromium.org
Related issue in Electron: https://github.com/electron/electron/issues/9173

I recently started to see some odd behavior when typing Japanese words on Mac Slack app. It might be related to this issue.

Comment 12 by nya@chromium.org, May 19 2017

Cc: yukawa@chromium.org kinaba@chromium.org
+kinaba, yukawa who reviewed the breaking change

Comment 13 by nya@chromium.org, May 20 2017

By removing the guard checking monitor_composition_info_ in RenderWidget::UpdateCompositionInfo(), the issue disappears.
https://cs.chromium.org/chromium/src/content/renderer/render_widget.cc?q=content/renderer/render_widget.cc&dr&l=1846

So I guess we do not send InputMsg_RequestCompositionUpdate while needed in some cases. <webview> creates a separate process so it might be related?

Comment 14 by nya@chromium.org, May 22 2017

Further looked at this issue with nona@ and kinaba@.

Some new observations:

1. The issue is not reproducible on Linux.

2. But even on Linux, IME-related internal behavior looks odd in <webview>.
While a text input is focused and the text cursor is blinking, RenderWidget::UpdateCompositionInfo() is called periodically. If a text box is in a simple web page, UpdateCompositionInfo() is not skipped because monitor_composition_info_ = true. However, if it is in <webview>, UpdateCompositionInfo() is *skipped* because monitor_composition_info_ = false.
This is true on Mac and Linux (Windows not tested), and looks suspicious.

3. RenderWidgetHostView::OnUpdateTextInputStateCalled() gets suspicious |updated_view|
I added this logging:
https://codereview.chromium.org/2896753002/diff/20001/content/browser/renderer_host/render_widget_host_view_aura.cc
https://codereview.chromium.org/2896753002/diff/20001/content/browser/renderer_host/render_widget_host_view_mac.mm
and checked which URL is shown. In normal web pages, its URL was shown as expected. In <webview>, webview's outer page URL was shown. I'm not sure the latter is expected or not.
This is true on Linux and Mac at least (Windows not tested).

Comment 15 by nya@chromium.org, May 22 2017

In fact, according to nona@, the change was actually targeted only for Android initially, but he was suggested to apply it to all platforms during review, and the platform restriction was removed while it was not well verified in other platforms than Android.

So, while I can continue investigation, I think we can just disable the optimization introduced in https://codereview.chromium.org/2121953002 only on Mac, especially since M60 branch cut is approaching very soon.

Cc: aelias@chromium.org
Components: UI>Input>Text>IME
> he was suggested to apply it to all platforms during review

+aelias@, who suggested this.

Note that IIRC this is the 3rd instance of regressions caused by that design.
 -  Bug 639552  [Mac]: Composition string window is sometimes misplaced for Japanese IME.
 -  Bug 640012  [Windows, CrOS]: CJK candidate windows appear in wrong place when using multiple IMEs

Comment 17 by nya@chromium.org, May 22 2017

Status: Started (was: Available)
I'd like to add a few points to this bug:

1- From what I see, changes in nona@'s patch did not include <webview>s. To support the feature for <webview> the InputMsg_RequestCompositionUpdates has to be sent to the guest process. The way input is routed to a <webview> is to first send it to its embedder process (that we do) and then forward it to BrowserPlugin (seems like we are missing this) which will then be sent back to browser (BrowserPluginGuest - still missing). BPG should be the right entity to send the request to the guest renderer.

2- With the chrome://flag 'Cross process frames for guests' enabled, this issue cannot be reproduced. This is because input routing for OOPIF-based guests is direct and currently any feature for IME for a webpage works exactly the same on those <webview>s. That being said, this experimental feature is under review to hopefully launch in M60. So perhaps if not urgent, we could wait for the new version of chrome with OOPIF-based guests.


3: But it might still be worthwhile fixing this issue on non-OOPIF guests given that MimeHandlerViewGuest (PDF viewer) will be based on older structure (BrowserPlugin) for a while.

Comment 19 by nya@chromium.org, May 22 2017

Thanks ekaramad@ for your insight!

In my opinion this issue is more severe than we expected first because of Electron's use of <webview>, so I really want to make sure it is fixed in M60. I sent a workaround CL https://codereview.chromium.org/2901443002/ which just temporarily disables the optimization on Mac. I'm glad if we can submit this workaround for M60 and later work on proper fixes.

ekaramad@, do you have bandwidth to work on a proper fix of this issue? None of nona@, kinaba@, yukawa@ are not working actively on Chrome IME today (and I'm a newbie) so any help is really appreciated.

Owner: ekaramad@chromium.org
Yes I can take a look later this week. Thanks!

Comment 22 by yosin@chromium.org, May 24 2017

Components: -Blink>Editing Blink>Editing>IME
We are seeing an issue in Electron with the proposed commit cherry-picked against Chrome 58, looks like the first character typed is sometimes incorrect. There is a GIF in https://github.com/electron/electron/issues/9709 that shows the behavior.

Any ideas what might be impacting this or what other commits may need to be cherry-picked? Thanks.
I have a CL in progress which will hopefully land soon. The commit will most probably be merged back to M60.

Here is the CL:
https://codereview.chromium.org/2929543002/.
Project Member

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

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

commit 2c3c487604c90c9ce96fefe0a32f660050f9fe55
Author: ekaramad <ekaramad@chromium.org>
Date: Thu Jun 15 14:58:06 2017

Request Composition Range Updates for Focused GuestViews based on BrowserPlugins

Since the patch https://codereview.chromium.org/2121953002/ landed, composition
range information are only requested for RenderWidgets which have a focused
text input box, that is, their TextInputState.type != ui::TEXT_INPUT_STATE_NONE.

This change however is not implemented for guest views which are based on
BrowserPlugin and therefore, some IME features in Japanese IME have regressed
for <webview>s. While this is not a bug in OOPIF-based guests, it is still an
important bug to fix because:
  1- PDFs will still use BrowserPlugin for a while,
  2- OOPIF-based <webview>'s might not launch in M60.

This patch will add the support by sending the request after BrowserPluginGuest
forwards a TextInputStateChanged call to RenderWidgetHostViewGuest.

BUG= 714771 

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

[modify] https://crrev.com/2c3c487604c90c9ce96fefe0a32f660050f9fe55/chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc
[modify] https://crrev.com/2c3c487604c90c9ce96fefe0a32f660050f9fe55/content/browser/browser_plugin/browser_plugin_guest.cc
[modify] https://crrev.com/2c3c487604c90c9ce96fefe0a32f660050f9fe55/content/browser/renderer_host/text_input_manager.cc
[modify] https://crrev.com/2c3c487604c90c9ce96fefe0a32f660050f9fe55/content/browser/renderer_host/text_input_manager.h
[modify] https://crrev.com/2c3c487604c90c9ce96fefe0a32f660050f9fe55/content/public/test/text_input_test_utils.cc
[modify] https://crrev.com/2c3c487604c90c9ce96fefe0a32f660050f9fe55/content/public/test/text_input_test_utils.h

Status: Fixed (was: Started)
Marking the bug as fixed following comment #25. I verified it on Chrome Canary
61.0.3134.0 (Official Build) canary (64-bit):

1- Open browser sample which embeds a <webview>.
(browser sample: https://chrome.google.com/webstore/detail/browser-sample/edggnmnajhcbhlnpjnogkjpghaikidaa)
2- Enable Japanese IME (Katakana) and type "w" inside <input> on google.com.
3- Press 'backspace' and observe that the character is removed.

(Alternatively, try the steps in bug description).
Labels: Merge-Request-60
Give that:
1- The fix is small,
2- OOPIF-based guests might not launch in M60
3- This is affecting Electron (high visibility)

Adding merge requests for M60.
Project Member

Comment 28 by sheriffbot@chromium.org, Jun 19 2017

Labels: -Merge-Request-60 Hotlist-Merge-Review Merge-Review-60
This bug requires manual review: M60 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-60 Merge-Approved-60
bug regression, based on comment 27, approving merge to m60. 
Project Member

Comment 30 by sheriffbot@chromium.org, Jun 26 2017

Cc: abdulsyed@chromium.org
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

Comment 31 by bugdroid1@chromium.org, Jun 26 2017

Labels: -merge-approved-60 merge-merged-3112
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/48285bdee11f3f1ed3ec16cbdbce5ddbd8681232

commit 48285bdee11f3f1ed3ec16cbdbce5ddbd8681232
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Mon Jun 26 18:22:53 2017

Request Composition Range Updates for Focused GuestViews based on BrowserPlugins

Since the patch https://codereview.chromium.org/2121953002/ landed, composition
range information are only requested for RenderWidgets which have a focused
text input box, that is, their TextInputState.type != ui::TEXT_INPUT_STATE_NONE.

This change however is not implemented for guest views which are based on
BrowserPlugin and therefore, some IME features in Japanese IME have regressed
for <webview>s. While this is not a bug in OOPIF-based guests, it is still an
important bug to fix because:
  1- PDFs will still use BrowserPlugin for a while,
  2- OOPIF-based <webview>'s might not launch in M60.

This patch will add the support by sending the request after BrowserPluginGuest
forwards a TextInputStateChanged call to RenderWidgetHostViewGuest.

BUG= 714771 

Review-Url: https://codereview.chromium.org/2929543002
Cr-Original-Commit-Position: refs/heads/master@{#479698}
Review-Url: https://codereview.chromium.org/2954963003 .
Cr-Commit-Position: refs/branch-heads/3112@{#466}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}

[modify] https://crrev.com/48285bdee11f3f1ed3ec16cbdbce5ddbd8681232/chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc
[modify] https://crrev.com/48285bdee11f3f1ed3ec16cbdbce5ddbd8681232/content/browser/browser_plugin/browser_plugin_guest.cc
[modify] https://crrev.com/48285bdee11f3f1ed3ec16cbdbce5ddbd8681232/content/browser/renderer_host/text_input_manager.cc
[modify] https://crrev.com/48285bdee11f3f1ed3ec16cbdbce5ddbd8681232/content/browser/renderer_host/text_input_manager.h
[modify] https://crrev.com/48285bdee11f3f1ed3ec16cbdbce5ddbd8681232/content/public/test/text_input_test_utils.cc
[modify] https://crrev.com/48285bdee11f3f1ed3ec16cbdbce5ddbd8681232/content/public/test/text_input_test_utils.h

Sign in to add a comment