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

Issue 767778 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression : In 'Edit bookmark' dialog, selected folder's name appears in black font color instead of white.

Reported by avsha...@etouch.net, Sep 22 2017

Issue description

Chrome version : 63.0.3222.0 (Official Build) 7754c77f5af465147885e57c6ba7fc4e9095d842-refs/heads/master@{#503582} 32/64 bit
OS : Windows (7,8,10)

What steps will reproduce the problem?
1. Launch chrome, open NTP and click on 'Bookmark this page' icon.
2. Click on 'Edit' button in bubble and select any 'Folder' in 'Edit bookmark' dialog.
3. Observe the font color of currently selected folder.

Actual Result : Selected folder's name appears in black font color. (black font does not look good with blue background color)

Expected Result : Instead, a currently selected folder's name should have white font color.

This is a regression issue broken in ‘M-63’, below is the manual bisect range and will soon update other info.
Good build : 63.0.3220.0
Bad build : 63.0.3221.0
 
Act_Exp_Result.png
14.2 KB View Download

Comment 1 by avsha...@etouch.net, Sep 22 2017

Labels: hasbisect-per-revision
Owner: ellyjo...@chromium.org
Status: Assigned (was: Unconfirmed)
Summary: Regression : In 'Edit bookmark' dialog, selected folder's name appears in black font color instead of white. (was: Regression : In 'Edit bookmark' dialog, selected folder's name appears in black font color.)
Using the per-revision bisect providing the bisect results,
Good build : 63.0.3220.0 (Revision : 502994)
Bad build : 63.0.3221.0 (Revision : 503308)

You are probably looking for a change made after 503164 (known good), but no later than 503165 (first known bad).

CHANGELOG URL: 
https://chromium.googlesource.com/chromium/src/+log/0beaddc405726742b5545de252e6bf0a2d43edba..efec5839674a3443f4648437c261456dfffc10d1

Suspect : https://chromium.googlesource.com/chromium/src/+/efec5839674a3443f4648437c261456dfffc10d1

@ellyjones : 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 : This is Windows OS specific issue and the same is not reproducible on Mac(10.12.6) and Linux(14.04 LTS) OS.

Thank you.
Labels: ReleaseBlock-Stable
Adding RB Label as this is a recent Regression. Please remove if not required.
Thank You.

Comment 3 by ajha@chromium.org, Sep 26 2017

Cc: sky@chromium.org
Cc'ing sky@ as well for inputs on this.
Labels: Needs-Triage-M63
Still seeing the same issue on latest Canary-63.0.3230.0.

ellyjones / sky, please take a look as it is marked as stable blocker.

Thanks.!

Looking at this today.
Status: Started (was: Assigned)
Ah, I figured this one out I think.

The old code was:

ui::NativeTheme::ColorId text_color_id(bool has_focus, bool is_selected) {
  if (is_selected) {
    if (has_focus)
      return ui::NativeTheme::kColorId_TreeSelectedText;
    return ui::NativeTheme::kColorId_TreeSelectedTextUnfocused;
  }
  return ui::NativeTheme::kColorId_TreeText;
}

the new code is:

SkColor TreeViewDrawingProvider::GetTextColorForNode(TreeView* tree_view,
                                                     ui::TreeModelNode* node) {
  ui::NativeTheme::ColorId color_id = ui::NativeTheme::kColorId_TreeText;
  if (tree_view->GetSelectedNode() == node) {
    if (tree_view->HasFocus())
      color_id = ui::NativeTheme::kColorId_TreeSelectedText;
    color_id = ui::NativeTheme::kColorId_TreeSelectedTextUnfocused; // <---- bug
  }
  return tree_view->GetNativeTheme()->GetSystemColor(color_id);
}

which erroneously always sets the color to TreeSelectedTextUnfocused even when tree_view->HasFocus(), which doesn't do the right thing on Windows. Those colors are the same on Linux and Mac, which is why I didn't notice this earlier. Fix will be up shortly.
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 6 2017

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

commit 9a8273052346c3c3c8c8312bdde31ba8236470ef
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Fri Oct 06 14:40:53 2017

views: use the right color for unfocused treeview selection

The existing code incorrectly always uses TreeSelectedTextUnfocused,
even when the TreeView is focused. Oops.

Bug:  767778 
Change-Id: I79cbb8628a607a10901cd744ab71e37ead85aa62
Reviewed-on: https://chromium-review.googlesource.com/703095
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507063}
[modify] https://crrev.com/9a8273052346c3c3c8c8312bdde31ba8236470ef/ui/views/controls/tree/tree_view_drawing_provider.cc

Status: Fixed (was: Started)
This should be fixed now, but I don't have a Windows machine to test it on, so please verify :)
Labels: TE-Verified-M63 TE-Verified-63.0.3236.0
Update : 
Retested above issue in latest chrome canary #63.0.3236.0 build on Windows(7,8,10) OS and issue is fixed. Now in 'Edit bookmark' dialog, selected folder's name is seen with white font color.
Kindly review an attached screen cast.

Thank you!
Canary_behavior.png
12.7 KB View Download

Sign in to add a comment