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

Issue 893384 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 19
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

First item is always highlighted when opening overview

Project Member Reported by mccanny@chromium.org, Oct 9

Issue description

Opening overview in tablet mode (but not laptop mode) always shows a highlight on the first item (See screencap: https://drive.google.com/file/d/1tNnVngX7v13gwOqFgdKo_ELTBIA-SvDc/view?usp=sharing).


 
Components: UI>Shell>OverviewMode
Labels: -Pri-3 M-71 OS-Chrome Pri-2
Bisected this to [1]. In particular enable-floating-virtual-keyboard will cause this. It seems with this flag on OnContentsChanged[2] gets triggered when the textfield is created, which adds the highlight.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1159556
[2] https://cs.chromium.org/chromium/src/ash/wm/overview/window_selector.cc?rcl=4c5f63ac3f3694369f816267127d31ead550580c&l=843
Cc: dvallet@chromium.org yhanada@chromium.org
+dvallet, yhanada

dvallet/yhanada: Any thoughts?
Cc: -x...@chromium.org sammiequon@chromium.org
Owner: x...@chromium.org
I ran into this issue today and did a bit investigation, and found when enable-floating-virtual-keyboard is enabled, Textfiled::UpdateAfterChange() will always be called, which then calls into WindowSelector::ContentsChanged with an empty content string to show the selection widget. We probably should do some special handling when the content is an empty string. I'll take a look.

FYI: This is the callstack 
#0  ash::WindowGrid::InitSelectionWidget(ash::WindowSelector::Direction) ()
    at ../../ash/wm/overview/window_grid.cc:1124
#1  0x0000589e163e25df in ash::WindowGrid::MoveSelectionWidget(ash::WindowSelector::Direction, bool, bool, bool)
    () at ../../ash/wm/overview/window_grid.cc:1183
#2  0x0000589e163e2328 in ash::WindowGrid::Move(ash::WindowSelector::Direction, bool) ()
    at ../../ash/wm/overview/window_grid.cc:462
#3  0x0000589e162f8fd9 in ash::WindowSelector::ContentsChanged(views::Textfield*, std::__1::basic_string<unsigned short, base::string16_internals::string16_char_traits, std::__1::allocator<unsigned short> > const&) ()
    at ../../ash/wm/overview/window_selector.cc:1044
#4  0x0000589e159130df in views::Textfield::UpdateAfterChange(bool, bool) ()
    at ../../ui/views/controls/textfield/textfield.cc:2125
#5  0x0000589e159172a3 in non-virtual thunk to views::Textfield::SetCompositionText(ui::CompositionText const&) ()
    at ../../ui/views/controls/textfield/textfield.cc:1444
#6  0x0000589e179b0611 in ui::InputMethodChromeOS::UpdateCompositionText(ui::CompositionText const&, unsigned int, bool) () at ../../ui/base/ime/input_method_chromeos.cc:620
#7  0x0000589e167216ec in input_method::InputMethodEngineBase::SetComposition(int, char const*, int, int, int, std::__1::vector<input_method::InputMethodEngineBase::SegmentInfo, std::__1::allocator<input_method::InputMethodEngineBase::SegmentInfo> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >*) () at ../../chrome/browser/ui/input_method/input_method_engine_base.cc:229
#8  0x0000589e15e134ed in extensions::InputImeSetCompositionFunction::Run() ()
    at ../../chrome/browser/extensions/api/input_ime/input_ime_api.cc:350
#9  0x0000589e129dcfb2 in ExtensionFunction::RunWithValidation() () at ../../extensions/browser/extension_function.cc:451
#10 0x0000589e129ded1c in extensions::ExtensionFunctionDispatcher::DispatchWithCallbackInternal(ExtensionHostMsg_Request_Params const&, content::RenderFrameHost*, int, base::RepeatingCallback<void (ExtensionFunction::ResponseType, base::ListValue const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, extensions::functions::HistogramValue)> const&) ()
    at ../../extensions/browser/extension_function_dispatcher.cc:486
#11 0x0000589e129de9a9 in extensions::ExtensionFunctionDispatcher::Dispatch(ExtensionHostMsg_Request_Params const&, content::RenderFrameHost*, int) () at ../../extensions/browser/extension_function_dispatcher.cc:404
#12 0x0000589e12a0079b in bool IPC::MessageT<ExtensionHostMsg_Request_Meta, std::__1::tuple<ExtensionHostMsg_Request_Params>, void>::Dispatch<extensions::ExtensionWebContentsObserver, extensions::ExtensionWebContentsObserver, content::RenderFrameHost, void (extensions::ExtensionWebContentsObserver::*)(content::RenderFrameHost*, ExtensionHostMsg_Request_Params const&)>(IPC::Message const*, extensions::ExtensionWebContentsObserver*, extensions::ExtensionWebContentsObserver*, content::RenderFrameHost*, void (extensions::ExtensionWebContentsObserver::*)(content::RenderFrameHost*, ExtensionHostMsg_Request_Params const&)) () at ../../ipc/ipc_message_templates.h:64
#13 0x0000589e12a006e7 in extensions::ExtensionWebContentsObserver::OnMessageReceived(IPC::Message const&, content::RenderFrameHost*) ()
    at ../../extensions/browser/extension_web_contents_observer.cc:248
#14 0x0000589e15da7f79 in extensions::ChromeExtensionWebContentsObserver::OnMessageReceived(IPC::Message const&, content::RenderFrameHost*) ()
    at ../../chrome/browser/extensions/chrome_extension_web_contents_observer.cc:105
#15 0x0000589e1281bed3 in content::WebContentsImpl::OnMessageReceived(content::RenderFrameHostImpl*, IPC::Message const&) ()
    at ../../content/browser/web_contents/web_contents_impl.cc:885
#16 0x0000589e1257d46b in content::RenderFrameHostImpl::OnMessageReceived(IPC::Message const&) ()
    at ../../content/browser/frame_host/render_frame_host_impl.cc:1206


Project Member

Comment 5 by bugdroid1@chromium.org, Oct 19

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

commit e5ab1b23536ec86f675c3c2987bb660af8c3ac0d
Author: Xiaoqian Dai <xdai@chromium.org>
Date: Fri Oct 19 23:09:19 2018

Don't show the selection widget for the first overview item if...

... the selection widget is already active or the filter string is empty.

Bug:  893384 
Change-Id: I793e083991ef60361a4ca3056347a4023919b240
Reviewed-on: https://chromium-review.googlesource.com/c/1292713
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601346}
[modify] https://crrev.com/e5ab1b23536ec86f675c3c2987bb660af8c3ac0d/ash/wm/overview/window_selector.cc

Labels: Merge-Request-71
Status: Fixed (was: Untriaged)
Project Member

Comment 7 by sheriffbot@chromium.org, Oct 20

Labels: -Merge-Request-71 Hotlist-Merge-Approved Merge-Approved-71
Your change meets the bar and is auto-approved for M71. Please go ahead and merge the CL to branch 3578 manually. Please contact milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

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

Comment 8 by bugdroid1@chromium.org, Oct 22

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/674cd1e5da9edc57203d0d75c18f244732a7f0fc

commit 674cd1e5da9edc57203d0d75c18f244732a7f0fc
Author: Xiaoqian Dai <xdai@chromium.org>
Date: Mon Oct 22 16:43:46 2018

[Merge to M71] Don't show the selection widget for the first overview item if...

... the selection widget is already active or the filter string is empty.

Bug:  893384 
TBR=xiyuan@chromium.org

Change-Id: I793e083991ef60361a4ca3056347a4023919b240
Reviewed-on: https://chromium-review.googlesource.com/c/1292713
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Xiaoqian Dai <xdai@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#601346}(cherry picked from commit e5ab1b23536ec86f675c3c2987bb660af8c3ac0d)
Reviewed-on: https://chromium-review.googlesource.com/c/1293998
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#218}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/674cd1e5da9edc57203d0d75c18f244732a7f0fc/ash/wm/overview/window_selector.cc

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/674cd1e5da9edc57203d0d75c18f244732a7f0fc

Commit: 674cd1e5da9edc57203d0d75c18f244732a7f0fc
Author: xdai@chromium.org
Commiter: xdai@chromium.org
Date: 2018-10-22 16:43:46 +0000 UTC

[Merge to M71] Don't show the selection widget for the first overview item if...

... the selection widget is already active or the filter string is empty.

Bug:  893384 
TBR=xiyuan@chromium.org

Change-Id: I793e083991ef60361a4ca3056347a4023919b240
Reviewed-on: https://chromium-review.googlesource.com/c/1292713
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Xiaoqian Dai <xdai@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#601346}(cherry picked from commit e5ab1b23536ec86f675c3c2987bb660af8c3ac0d)
Reviewed-on: https://chromium-review.googlesource.com/c/1293998
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#218}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

Sign in to add a comment