Issue metadata
Sign in to add a comment
|
Regression: Favicon icon is not seen for bookmarked page in 'Other bookmarks' folder.
Reported by
db...@etouch.net,
Nov 21 2017
|
||||||||||||||||||||||
Issue descriptionChrome Version: 64.0.3274.0 Revision 38c9257b930f883c3ab936b1274590c94cc08012-refs/heads/master@{#518061} OS: Windows(7,8,10), Linux(14.1LTS) What steps will reproduce the problem? (1) launch chrome, open NTP and bookmarked it by using 'Other bookmarks' folder. (2) Close browser and open it again, click on 'Other bookmarks' folder. (3) Observe favicon icon for bookmarked page. Actual: Favicon icon is not seen for bookmarked page. Expected: Favicon icon should seen for bookmarked page. This is a regression issue, broken in 'M64', below is bisect info: Good Build: 64.0.3262.0 Bad Build: 64.0.3263.0 You are probably looking for a change made after 514933 (known good), but no lat er than 514934 (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/3bc820ae7732bd0580a0e74aad8e04a1b2be0467..55b3fe20e2e1c5586d54004447d2953fee767599 Suspect: https://chromium.googlesource.com/chromium/src/+/55b3fe20e2e1c5586d54004447d2953fee767599 Note: Issue is not seen on Mac OS.
,
Nov 21 2017
Thx! I'll look into this. The culprit CL is accurate, and the feature behind a Finch flag that we can turn off. I'll look into whether a fix is simple.
,
Nov 21 2017
Just to clarify, this regression affects pages that do not have a favicon.
,
Nov 21 2017
,
Nov 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e7a247abf169ca5f2c2abca7787be0f8c19bc26c commit e7a247abf169ca5f2c2abca7787be0f8c19bc26c Author: Mikel Astiz <mastiz@chromium.org> Date: Tue Nov 28 12:56:03 2017 Fix bookmarks without favicon missing the default icon Bug introduced in 55b3fe20e2e1c5586d54004447d2953fee767599 where, for the first time, bookmark observers can get notified with an empty favicon (i.e. the bookmark does not have a favicon). In this case, the UIs that want to fall back to the default icon need to handle it specifically. BookmarkMenuDelegate seems to be the only UI that cares are this particular event. To fix it, we mimic the logic implemented in the very same class, in BookmarkMenuDelegate::BuildMenu(), and treat the null image just like the default icon. Bug: 787363 Change-Id: Idf8fc5d2f2937ab16780a43320dd6689876244fa Reviewed-on: https://chromium-review.googlesource.com/782320 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Mikel Astiz <mastiz@chromium.org> Cr-Commit-Position: refs/heads/master@{#519662} [modify] https://crrev.com/e7a247abf169ca5f2c2abca7787be0f8c19bc26c/chrome/browser/ui/views/bookmarks/bookmark_menu_delegate.cc
,
Nov 28 2017
The patch above should fix the reported issue. dbote@: could you please verify? |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by msrchandra@chromium.org
, Nov 21 2017