Regression: Blink of grey text selection color is seen on reloading page.
Reported by
rk...@etouch.net,
May 10 2016
|
|||||
Issue descriptionChrome version : 52.0.2730.0 (Official Build) 3a85b1c8884d4a28f86a69df86183b51018b8245-refs/heads/master@{#392504} 32/64 bit OS : Windows (7, 8, 10), Linux (14.04 LTS) What steps will reproduce the problem? (1) Launch chrome and navigate to chrome://version. (2) Press ctrl+A for select all and click on Reload icon, observe the text selection color. Blink of grey text selection color is seen on reloading page. Blink of grey text selection color should not seen on reloading page. This is a regression issue,broken in 'M-52', below is bisect info: Good Build: 52.0.2725.0 Bad Build: 52.0.2726.0 Narrow Bisect: https://chromium.googlesource.com/chromium/src/+log/8a350376c9d1d5d00fe1e8d810d0d5167f374c1d..f00e71eb761f55d65e770fcf3dec1c7a28c0ec3f?pretty=fuller&n=100 Suspecting: r391746 ? @michaelpg: Please help me to reassign this issue if your change is not cause for it. Note: Issue is not seen on Mac OS.
,
May 10 2016
That regression range doesn't seem right, as there is nothing in that change range that would seem to affect painting or selection on navigation. We seem to be repainting the selection for the first frame after the reload, sometimes but not always. Odd.
,
May 10 2016
,
May 11 2016
Pressing reload is sometimes causing the window focus state to change before reload, which causes the change in selection color (selections go gray when the window is not focused). We should ideally fix that to never cause the window to become unfocused when clicking on the reload button. I agree that this regression range doesn't make a lot of sense. My best guess is https://chromium.googlesource.com/chromium/src/+/24c156252c11425af51d646cd0d405f018c70bb8
,
May 11 2016
,
May 11 2016
Yeah this is related my change. What happens is when we click on the reload button, it gains focus(it should not), which causes the selected text to become grey until the reload completes. A fix for this is under review - https://codereview.chromium.org/1963563002/
,
May 16 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8c6b2ee186d786623f0e4cb6d05c08c934c0ff9b commit 8c6b2ee186d786623f0e4cb6d05c08c934c0ff9b Author: karandeepb <karandeepb@chromium.org> Date: Mon May 16 01:14:44 2016 Views: Change View::RequestFocus to respect keyboard accessibility. This CL fixes some regressions introduced in http://crrev.com/1894383002/. These regression are caused due to the change in View::RequestFocus() from IsFocusable() to IsAccessibilityFocusable(). On a mouse click on a CustomButton, CustomButton::MousePressed() requests focus on the button, if it has request_focus_on_press_ set to true. It turns out that most button subclasses, do not explicitly set request_focus_on_press_ to false, which has a default value of true. These custom buttons which are accessibility focusable, can now gain focus on a mouse press, hence the bug. This CL changes View::RequestFocus to use IsFocusable when keyboard accessibility is off (i.e on Non-Mac platforms), hence fixing bugs 609701 , 610186, 610235, 610740, 610802, 610664. This is how View::RequestFocus behaved before crrev.com/1894383002 on Non-Mac platforms. Also, on Mac, since View::RequestFocus now respects keyboard accessibility, bug 611280 is also fixed. BUG= 609701 , 610186 , 610235 , 610740 , 610802 , 610664 , 564912 , 611280 Review-Url: https://codereview.chromium.org/1973073003 Cr-Commit-Position: refs/heads/master@{#393781} [modify] https://crrev.com/8c6b2ee186d786623f0e4cb6d05c08c934c0ff9b/chrome/browser/ui/views/tabs/tab_unittest.cc [modify] https://crrev.com/8c6b2ee186d786623f0e4cb6d05c08c934c0ff9b/ui/views/focus/focus_manager.cc [modify] https://crrev.com/8c6b2ee186d786623f0e4cb6d05c08c934c0ff9b/ui/views/focus/focus_manager_unittest.cc [modify] https://crrev.com/8c6b2ee186d786623f0e4cb6d05c08c934c0ff9b/ui/views/view.cc
,
May 16 2016
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by ashej...@chromium.org
, May 10 2016