Regression : Unable to type anything in Google doc even if defaut text caret is present.
Reported by
avsha...@etouch.net,
Jan 16 2018
|
|||||||||||||
Issue descriptionChrome Version : 65.0.3322.3 (Official Build) 8758ca55b13d4f2082b2ed9269fce8f37f37c577-refs/branch-heads/3322@{#6} 32/64-bit OS : Mac(10.12.6, 10.13.1, 10.13.3), Windows(7,8,8.1,10), Linux(14.04 LTS) Test URL : https://www.google.com/intl/en/drive/ What steps will reproduce the problem? 1. Launch chrome, navigate to above URL and sign in to Google drive with valid credentials. 2. In google drive, click on ‘NEW’ button and select ‘Google Docs’ option (A fresh document opens in a new tab). 3. Let the document load completely (do not click anywhere on the document) and try to type something in it. 4. Observe. Actual Results : 1. Default text caret in google doc stops blinking after few seconds. 2. Unable to type anything in Google doc. Expected Results : 1. Default text caret in google docs should keep blinking so that it acknowledges user about the current focus position on a doc. 2. Should be able to type in Google doc. This is a regression issue broken in ‘M-65’ and using the per-revision bisect providing the bisect results, Good Build : 65.0.3295.0 (Revision : 524285) Bad Build : 65.0.3296.0 (Revision : 524554) You are probably looking for a change made after (known good), but no later than (first known bad). CHANGELOG URL: https://chromium.googlesource.com/chromium/src/+log/24abf78274e01a17c4f0a4c8cc18e55720f3b3b9..3b60933b099bba43823b2c5cbdbe7c3f2e90ae68 Suspect : https://chromium.googlesource.com/chromium/src/+/3b60933b099bba43823b2c5cbdbe7c3f2e90ae68 @alexmos : Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner. Note : 1. User needs to click twice on a google doc in order to make default text caret blink again. 2. Able to type in google doc, once the text caret start to blink again. 3. This issue is also observed on M-65 Dev #65.0.3315.3 build.
,
Jan 16 2018
Confirmed that this is due to sign-in isolation. I've verified this also affects the current stable release (63.0.3239.132 on Mac). It also occurs when disabling --site-per-process or --isolate-origins policies, but it does not occur when I run Chrome with --disable-features=sign-in-process-isolation. If you open the task manager, it looks like the caret freezes when the accounts.google.com subframe first appears in task manager. So I suspect it has something to do with the accounts subframe stealing focus from the Docs frame. I'll investigate. Clicking on the doc, or focusing another window and going back, unfreezes the caret and lets the user type. So there's an easy workaround, but it's still not a great user experience for sure. Note that sign-in isolation is currently enabled for 100% of all channels right now, so it's not a recent regression.
,
Jan 16 2018
,
Jan 17 2018
,
Jan 17 2018
I think I understand what's going on here. Fix in progress. This is a generic site isolation bug that just happens to manifest itself with sign-in isolation in Docs. Docs apparently puts the blinking caret in another iframe and focuses that frame with window.focus(). Then, it adds an accounts.google.com subframe (oauth-related, since it's named similar to oauth2relay317124780), which goes into an OOPIF when sign-in isolation is enabled. When that subframe commits, it triggers this code in RFHM::CommitPending(): // The main frame's view is already focused, but we need to set // page-level focus in the subframe's renderer. frame_tree_node_->frame_tree()->SetPageFocus( render_frame_host_->GetSiteInstance(), true); which sends an IPC to set page focus to true in the accounts.google.com renderer. However, at this point, that renderer doesn't know what the focused frame is and that it has already been changed from the main frame to the "blinking caret" iframe (both RemoteFrames in its process). It hits this code as part of setting page focus in FocusController::SetFocused(): if (!focused_frame_ && is_focused_) SetFocusedFrame(page_->MainFrame()); That sets the main frame as the focused frame and sends the FrameHostMsg_FrameFocused IPC up to the browser process, which resets the focused frame to the main frame and steals focus away from the blinking caret frame. A solution would be to let new OOPIF renderer processes know which RemoteFrame has focus in such cases, before setting page focus.
,
Jan 17 2018
I was testing this yesterday as part of another accessibility bug b/71605481, but I can verify that it is not an accessibility specific bug. This repros intermittently in many versions of Chrome (Canary, 65,64,63) with accessibility mode disabled and no screen readers. I tried to bisect across different ranges but the issue was too intermittent to tie to a specific Chrome release.
,
Jan 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f1d9af002af9478cb193b775b9b5889455c67e51 commit f1d9af002af9478cb193b775b9b5889455c67e51 Author: Alex Moshchuk <alexmos@chromium.org> Date: Thu Jan 18 00:54:24 2018 Prevent an OOPIF commit from stealing focus from other subframes. When a cross-process subframe navigation commits, the browser process sends an IPC to the OOPIF to tell it whether it currently has page focus (based on the page's overall page focus). However, it never tells the OOPIF about frame focus, in the case that some frame had become focused prior to that navigation. This leads to a bug, where as part of setting page focus, Blink's FocusController::SetFocused() will set the focused frame to the main frame if the focused frame isn't set yet, which will steal focus from subframes and give it to the main frame. This CL fixes this by telling OOPIF processes about both frame and page focus at commit time. Bug: 802156 Change-Id: I14a3cdc248681178b9a5eaf570310224aec2c887 Reviewed-on: https://chromium-review.googlesource.com/871523 Commit-Queue: Alex Moshchuk <alexmos@chromium.org> Reviewed-by: Charlie Reis <creis@chromium.org> Cr-Commit-Position: refs/heads/master@{#529989} [modify] https://crrev.com/f1d9af002af9478cb193b775b9b5889455c67e51/content/browser/frame_host/render_frame_host_manager.cc [modify] https://crrev.com/f1d9af002af9478cb193b775b9b5889455c67e51/content/browser/site_per_process_browsertest.cc
,
Jan 18 2018
,
Jan 18 2018
,
Jan 18 2018
Thanks Alex for the quick fix! Please follow b/71605481 to get the latest updates from Docs side on the fix.
,
Jan 18 2018
moving to RBS for M64 since user able to type after clicking in the doc, please add merge-request once CL is verified in ToT
,
Jan 18 2018
Commit f1d9af00... initially landed in 65.0.3324.0. People impacted by this bug have confirmed in b/71605481#comment18 that this bug no longer repros in today's Canary - I think we can mark the bug as fixed and request a merge to M64.
,
Jan 18 2018
This bug requires manual review: We are only 4 days from stable. Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 18 2018
I've also confirmed that the fix is working as intended on 65.0.3324.0, and I also checked that so far there aren't any related crashes being reported. The fix itself is small and well-contained (just a few lines in one function), so I think it's safe for merge.
,
Jan 18 2018
Approving merge for M64. Branch:3282
,
Jan 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/59d85e7d4bb4d4142a3bf9dae5e2611bb8400fc8 commit 59d85e7d4bb4d4142a3bf9dae5e2611bb8400fc8 Author: Alex Moshchuk <alexmos@chromium.org> Date: Fri Jan 19 00:01:44 2018 Prevent an OOPIF commit from stealing focus from other subframes (Merge to M64) When a cross-process subframe navigation commits, the browser process sends an IPC to the OOPIF to tell it whether it currently has page focus (based on the page's overall page focus). However, it never tells the OOPIF about frame focus, in the case that some frame had become focused prior to that navigation. This leads to a bug, where as part of setting page focus, Blink's FocusController::SetFocused() will set the focused frame to the main frame if the focused frame isn't set yet, which will steal focus from subframes and give it to the main frame. This CL fixes this by telling OOPIF processes about both frame and page focus at commit time. TBR=alexmos@chromium.org (cherry picked from commit f1d9af002af9478cb193b775b9b5889455c67e51) Bug: 802156 Change-Id: I14a3cdc248681178b9a5eaf570310224aec2c887 Reviewed-on: https://chromium-review.googlesource.com/871523 Commit-Queue: Alex Moshchuk <alexmos@chromium.org> Reviewed-by: Charlie Reis <creis@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#529989} Reviewed-on: https://chromium-review.googlesource.com/875313 Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Cr-Commit-Position: refs/branch-heads/3282@{#543} Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840} [modify] https://crrev.com/59d85e7d4bb4d4142a3bf9dae5e2611bb8400fc8/content/browser/frame_host/render_frame_host_manager.cc [modify] https://crrev.com/59d85e7d4bb4d4142a3bf9dae5e2611bb8400fc8/content/browser/site_per_process_browsertest.cc
,
Jan 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/665a1fc46d26c39dc613544c9b8a7c3b0fe76b6e commit 665a1fc46d26c39dc613544c9b8a7c3b0fe76b6e Author: Alex Moshchuk <alexmos@chromium.org> Date: Fri Jan 19 01:45:37 2018 Fix focus-related comment in RFHM::CommitPending(). This is a followup to address https://chromium-review.googlesource.com/c/chromium/src/+/871523/3/content/browser/frame_host/render_frame_host_manager.cc#2036 Bug: 802156 Change-Id: I1f261f4e0ca26e8c700a13ad15a76027d681d39c Reviewed-on: https://chromium-review.googlesource.com/875243 Reviewed-by: Charlie Reis <creis@chromium.org> Commit-Queue: Alex Moshchuk <alexmos@chromium.org> Cr-Commit-Position: refs/heads/master@{#530383} [modify] https://crrev.com/665a1fc46d26c39dc613544c9b8a7c3b0fe76b6e/content/browser/frame_host/render_frame_host_manager.cc
,
Jan 23 2018
I've verified the fix on 64.0.3282.113 Win beta, which just came out and includes the merge from #16.
,
Jan 24 2018
Verified the fix on Chrome 65.0.3325.9/10323.1.0(Peppy,Paine), 64.0.3282.114/10176.59.0(Peppy, Caroline)
,
Jan 25 2018
,
Feb 2 2018
|
|||||||||||||
►
Sign in to add a comment |
|||||||||||||
Comment 1 by nyerramilli@chromium.org
, Jan 16 2018