Issue metadata
Sign in to add a comment
|
macOS: Body scroll bar uses incorrect color on dark background |
||||||||||||||||||||||
Issue description
Chrome Version: 56.0.2924.87
OS: macOS 10.12.1
What steps will reproduce the problem?
1. Be on a Mac with system preferences set to use overlay scroll bars ("Show scroll bars" set to "when scrolling").
2. Launch Chrome
3. Load https://jsfiddle.net/dvgnsLev/
4. Scroll in the area to the right of the green line before the background color changes from black to white for the first time.
What is the expected result?
The right area should show a scroll bar for the body that has a gray drag region.
What happens instead?
The right area does not show the gray drag region until after the background color changes from black to white for the first time.
Worked as expected in 55.0.2883.95.
,
Feb 3 2017
ymalik@ seems inactive. bokan@ can you take a look?
,
Feb 6 2017
Over to chaopeng@ who has worked on the background color adjustment recently.
,
Feb 6 2017
Correction to #1: The suspected revision should be https://chromium.googlesource.com/chromium/src/+/7a119e4a1eef6d3a91e13b8b86cc12f6ca9b8804.
,
Feb 6 2017
There is an ordering issue for this. In |FrameView::ScrollbarManager::setHasVerticalScrollbar| we call |didAddScrollbar| before |m_vBarIsAttached = 1|. That makes |ScrollableArea::setScrollbarOverlayColorTheme| can not get the scrollbar to it's scrollbarTheme to setOverlayColorTheme. Why it only happens on Mac? Because we don't set the OverlayColorTheme on theme but get it from scrollbar for each paint. I suggest we should just use one place to record OverlayColorTheme.
,
Feb 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b45a817c9a86d2d25ed6fba359abceafc728c2d3 commit b45a817c9a86d2d25ed6fba359abceafc728c2d3 Author: chaopeng <chaopeng@chromium.org> Date: Wed Feb 08 00:18:28 2017 Set attached flag before we call didAddScrollbar This issue is caused by we call didAddScrollbar before set the attached flag. That makes FrameView::horizontalScrollbar, FrameView::verticalScrollbar always return null and ScrollableArea::setScrollbarOverlayColorTheme can not set OverlayColorTheme to ScrollbarThemeMac. In this patch we move the attached flag set before didAddScrollbar call. BUG= 688516 Review-Url: https://codereview.chromium.org/2671323004 Cr-Commit-Position: refs/heads/master@{#448814} [modify] https://crrev.com/b45a817c9a86d2d25ed6fba359abceafc728c2d3/third_party/WebKit/Source/core/frame/FrameView.cpp
,
Feb 8 2017
@comment#6: Thanks, works for me when cherry-picked to a local build of M56. Can this change be merged to M57?
,
Feb 8 2017
Yes, Chao, please confirm the fix in Canary and add Merge-Request label if confirmed?
,
Feb 8 2017
confirmed it works in canary
,
Feb 8 2017
,
Feb 9 2017
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions. Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Feb 9 2017
Please merge your change to M57 branch 2987 before 5:00 PM PT, Friday 02/10 (sooner the better please) so we can take it in for next week beta release. Thank you.
,
Feb 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f8c54e0176fb6c3f9b31e5dbfcfd8d727df8d500 commit f8c54e0176fb6c3f9b31e5dbfcfd8d727df8d500 Author: bokan <bokan@chromium.org> Date: Thu Feb 09 16:16:55 2017 Set attached flag before we call didAddScrollbar This issue is caused by we call didAddScrollbar before set the attached flag. That makes FrameView::horizontalScrollbar, FrameView::verticalScrollbar always return null and ScrollableArea::setScrollbarOverlayColorTheme can not set OverlayColorTheme to ScrollbarThemeMac. In this patch we move the attached flag set before didAddScrollbar call. BUG= 688516 Review-Url: https://codereview.chromium.org/2671323004 Cr-Commit-Position: refs/heads/master@{#448814} (cherry picked from commit b45a817c9a86d2d25ed6fba359abceafc728c2d3) Review-Url: https://codereview.chromium.org/2686713005 . Cr-Commit-Position: refs/branch-heads/2987@{#407} Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943} [modify] https://crrev.com/f8c54e0176fb6c3f9b31e5dbfcfd8d727df8d500/third_party/WebKit/Source/core/frame/FrameView.cpp
,
Feb 9 2017
Patch merged back to M57. Closing.
,
Feb 15 2017
Verified on mac os 10.12.2 using chrome beta M57 #57.0.2987.54 and issue is fixed. Steps followed : 1. Set the system preferences -> show scrollbars to "when scrolling ". 2. Launched chrome and navigated to https://jsfiddle.net/dvgnsLev/ 3. Scrolled on the right side of the green line and observed the grey scorllbar . Attached screencast for reference. Adding TE-Verified labels. Thanks! |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by wangxianzhu@chromium.org
, Feb 3 2017Status: Assigned (was: Untriaged)