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

Issue 779983 link

Starred by 2 users

Issue metadata

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

Blocking:
issue 786300



Sign in to add a comment

Regression:Unnecessary blue highlight is seen on folder after pressing shift+tab key.

Reported by vku...@etouch.net, Oct 31 2017

Issue description

Chrome Version:64.0.3253.3 (Official Build) d5f4ba12a6c29b1b49da50663e51d7d6d2dad88e-refs/branch-heads/3253@{#5} (32/64-bit) 
OS: Windows (7,8,10)

What steps will reproduce the problem?
(1)Launch chrome and navigate to any long url, copy the url
(2)Press ctrl+shift+B , right click on bookmark bar > add folder > new folder and paste the long url OR enter long name such that scroll bar appears.
(3)Right click on it > edit, scroll towards extreme right and press shift+tab key, observe the blue highlight.

Actual: Unnecessary blue highlight is seen on folder after pressing shift+tab key. 

Expected: Blue highlight should not be seen on folder after pressing shift+tab key.

This is a regression issue broken in 'M63' and will soon update other info.

Note: Issue not seen on Mac & Linux OS.
 
Actual_Bookmark.mp4
523 KB View Download
Expected_Bookmark.mp4
694 KB View Download

Comment 1 by vku...@etouch.net, Oct 31 2017

Labels: hasbisect-per-revision
Owner: ellyjo...@chromium.org
Status: Assigned (was: Unconfirmed)
Good Build: 63.0.3220.0 
Bad Build:  63.0.3221.0

You are probably looking for a change made after 503164 (known good), but no lat
er than 503165 (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/0beaddc405726742b5545de252e6bf0a2d43edba..efec5839674a3443f4648437c261456dfffc10d1

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

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.
Cc: bsep@chromium.org
It is possible that my change caused this. I don't have Windows hardware to debug it on. Shift-Tab should be moving focus to the previous input area, so perhaps this is a problem with the area which gets scheduled for repaint when TreeView loses focus.

bsep@: do you happen to have Windows hardware handy?
Labels: ReleaseBlock-Stable
Tagging with blocker label, please undo if not the case.

Thanks.!

Comment 4 by bsep@chromium.org, Oct 31 2017

Cc: -bsep@chromium.org ellyjo...@chromium.org
Owner: bsep@chromium.org
Sure, I'll take a look.
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 4 2017

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

commit a7ad077437ea1ae74cf4c19aab93723c7230f7ad
Author: Bret Sepulveda <bsep@chromium.org>
Date: Fri Nov 03 23:00:55 2017

Fix selected rows that scroll in a TreeView having incorrect damage.

When the TreeView scheduled a paint it was using the wrong bounds when
the platform wanted to only highlight the row's text (i.e. on Windows).
This resulted in some areas not being repainted when a long selected row
scrolls to the right (in LTR).

Bug:  779983 
Change-Id: I5210d5337a7ad2a6ae6dbaf85ba2601862c7accd
Reviewed-on: https://chromium-review.googlesource.com/748209
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513931}
[modify] https://crrev.com/a7ad077437ea1ae74cf4c19aab93723c7230f7ad/ui/views/controls/tree/tree_view.cc

Comment 6 by vku...@etouch.net, Nov 6 2017

Labels: TE-Verified-M64 TE-Verified-64.0.3260.0
Retested above issue on Windows(7,8,10) OS using latest Canary #64.0.3260.0 and issue is fixed. Kindly review an attached screen cast.
Actual.mp4
609 KB View Download
M63 Stable promotion is coming soon and your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge to M63 ASAP. Thank you.

Comment 8 by bsep@chromium.org, Nov 6 2017

Labels: Merge-Request-63
Project Member

Comment 9 by sheriffbot@chromium.org, Nov 6 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Before we approve merge to M63 branch 3239, could you pls confirm change is well baked in Canary and safe to merge to M63?

Comment 11 by bsep@chromium.org, Nov 6 2017

#10: Yes, I can confirm it's working in Canary. The change is very small and safe to merge imo.
Labels: -Merge-Review-63 Merge-Approved-63
Approving merge to M63 branch 3239 based on comments #6 and #11. Please merge ASAP. Thank you.
Project Member

Comment 13 by bugdroid1@chromium.org, Nov 6 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d9ba4ef60ea8ea8e3ef7e7dc6cc2b4ab798014ae

commit d9ba4ef60ea8ea8e3ef7e7dc6cc2b4ab798014ae
Author: Bret Sepulveda <bsep@chromium.org>
Date: Mon Nov 06 21:05:22 2017

Fix selected rows that scroll in a TreeView having incorrect damage.

When the TreeView scheduled a paint it was using the wrong bounds when
the platform wanted to only highlight the row's text (i.e. on Windows).
This resulted in some areas not being repainted when a long selected row
scrolls to the right (in LTR).

TBR=bsep@chromium.org

(cherry picked from commit a7ad077437ea1ae74cf4c19aab93723c7230f7ad)

Bug:  779983 
Change-Id: I5210d5337a7ad2a6ae6dbaf85ba2601862c7accd
Reviewed-on: https://chromium-review.googlesource.com/748209
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#513931}
Reviewed-on: https://chromium-review.googlesource.com/755517
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#396}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/d9ba4ef60ea8ea8e3ef7e7dc6cc2b4ab798014ae/ui/views/controls/tree/tree_view.cc

Comment 14 by bsep@chromium.org, Nov 6 2017

Status: Fixed (was: Assigned)

Comment 15 by vku...@etouch.net, Nov 7 2017

Labels: TE-Verified-M63 TE-Verified-63.0.3239.39
Note:
Rechecked the above issue on Beta #63.0.3239.39 on Windows (7,8,10) and fix is working as intended.

Please refer attached screencast
Actual Beta.mp4
374 KB View Download
Blocking: 786300
Project Member

Comment 17 by bugdroid1@chromium.org, Nov 20 2017

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

commit a6d3776aae145775e774979f7226470dbca284a9
Author: Scott Violet <sky@chromium.org>
Date: Mon Nov 20 20:02:29 2017

views: fix bug in tree node bounds calculation

A couple of bounds calculations used bounds().x(). bounds() is
relative to the parent, where as these calculations are meant to be
relative to the tree. So, they shouldn't use bounds().x() and should
instead always be 0 based.

BUG= 779983 
TEST=none

Change-Id: I92ffd99c9c7caef25b06bb7bdf3c05bbae466775
Reviewed-on: https://chromium-review.googlesource.com/777768
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517897}
[modify] https://crrev.com/a6d3776aae145775e774979f7226470dbca284a9/ui/views/controls/tree/tree_view.cc

Is cl listed at #17 need to be merged to M63?

Comment 19 by vku...@etouch.net, Nov 21 2017

Labels: TE-Verified-64.0.3274.0
Rechecked the above issue on latest Canary #64.0.3274.0 on Windows (7,8,10) and fix is working as intended.

Please refer attached screencast

Actual.mp4
762 KB View Download

Sign in to add a comment