Issue metadata
Sign in to add a comment
|
Regression: Unwanted close icon ring focus of 1st tab is seen overlapped on 2nd tab while dragging
Reported by
khushal....@etouch.net,
Jun 18 2018
|
||||||||||||||||||||||||
Issue descriptionChrome Version : 69.0.3464.0 (Official Build)Revision 3c26b60e3842fee660bcff5eb35aa0587d795f02-refs/branch-heads/3464@{#1} (64 bit) OS: Mac (10.12.6, 10.13.1, 10.13.5, 10.13.6) Pre-condition: Enable "Use Views browser windows instead of Cocoa." flag in chrome://flags Steps to reproduce: 1. Launch chrome and open 2 Tabs. 2. Now drag 2nd tab to the 1st tab position and Observe the ring focus of Close icon. Actual Result: Unwanted close icon ring focus of 1st tab is seen overlapped on 2nd tab while dragging. Expected Result: Close icon ring focus of 1st tab should not be seen over 2nd tab. This is Regression issue broken in 'M-69' and providing the bisect info below: Good Build: 69.0.3453.0 (Revision: 565532) Bad Build: 69.0.3455.0 (Revision: 565999) You are probably looking for a change made after 565672 (known good), but no later than 565673 (first known bad). CHANGELOG URL: The script might not always return single CL as suspect as some perf builds might get missing due to failure. https://chromium.googlesource.com/chromium/src/+log/7aebaa45cb7ddc98c7e052f3fdaa0efe62627c95..a0c4aeac1add95b255d9adaa583969b6dc88eb7c Suspect: https://chromium.googlesource.com/chromium/src/+/a0c4aeac1add95b255d9adaa583969b6dc88eb7c @ellyjones: 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. Note: Issue is not seen on Windows (7, 8, 8.1, 10) & Linux (14.04 LTS) OS. Kindly refer attached screen-cast. Thank You..!!
,
Jul 3
,
Jul 6
The NextAction date has arrived: 2018-07-06
,
Jul 6
I reproduced this locally. I'm surprised that the tab's close button takes focus during a tab drag - I'm going to investigate why that is. Assuming it's the desired behavior, I'll need to instead add a hack to suppress the focus rings while the tab is in a drag, but I'm worried that that will cause the focus ring to flicker at the start and end of drags.
,
Jul 6
Hmm. This is how the first tab's focus ring gets focus when the drag starts: 1 libchrome_dll.dylib 0x000000010376fd50 TabCloseButton::OnFocus() + 32 2 libviews.dylib 0x000000010e02b810 views::View::Focus() + 32 3 libviews.dylib 0x000000010e00f616 views::FocusManager::SetFocusedViewWithReason(views::View*, views::FocusManager::FocusChangeReason) + 486 4 libviews.dylib 0x000000010e00f39c views::FocusManager::AdvanceFocus(bool) + 188 5 libviews.dylib 0x000000010e00fdbb views::FocusManager::AdvanceFocusIfNecessary() + 75 6 libviews.dylib 0x000000010e010032 views::FocusManager::RestoreFocusedView() + 130 7 libviews.dylib 0x000000010df9598a -[BridgedContentView becomeFirstResponder] + 90 8 AppKit 0x00007fff33dd1dac -[NSWindow _realMakeFirstResponder:] + 527 9 libviews.dylib 0x000000010e02b7c0 views::View::OnFocus() + 32 10 libviews.dylib 0x000000010e02b810 views::View::Focus() + 32 11 libviews.dylib 0x000000010e00f616 views::FocusManager::SetFocusedViewWithReason(views::View*, views::FocusManager::FocusChangeReason) + 486 12 libchrome_dll.dylib 0x0000000103771485 TabDragController::SaveFocus() + 149 13 libchrome_dll.dylib 0x0000000103770f7e TabDragController::Drag(gfx::Point const&) + 286
,
Jul 6
TabStripController::SaveFocus is doing:
void TabDragController::SaveFocus() {
DCHECK(source_tabstrip_);
old_focused_view_tracker_->SetView(
source_tabstrip_->GetFocusManager()->GetFocusedView());
source_tabstrip_->GetFocusManager()->SetFocusedView(source_tabstrip_);
// WARNING: we may have been deleted.
}
So it tries to hand keyboard focus off to the tabstrip, which calls into View::OnFocus(), which ultimately makes the BridgedContentView the first responder again. BridgedContentView was not the first responder before because the WebContents was. When the BridgedContentView becomes first responder, it calls FocusManager::RestoreFocusedView() to restore the Views-side focus. That calls AdvanceFocusIfNecessary() in case the FKA setting has changed.
Evidently !FocusManager::IsFocusable(source_tabstrip_), and then we end up traversing to the first close button. It makes sense that the tabstrip itself is not keyboard-focusable.
Still investigating how to perhaps fix this.
,
Jul 6
lgrey@ pointed me at the related issue 835550 . I think the correct behavior here is for the *omnibox* to receive keyboard focus during a drag, not the tab strip.
,
Jul 6
Discussion of this has been subsumed by issue 835550 , which is basically the same bug. |
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by rbasuvula@chromium.org
, Jun 18 2018