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

Issue 792605 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug

Blocking:
issue 671916



Sign in to add a comment

[macViewsBrowser] Tab favicons are missing

Project Member Reported by shrike@chromium.org, Dec 6 2017

Issue description

Chrome Version: 65.0.3287.0
OS: macOS 10.12

The loading spinner appears, but after it finishes there's a blank space where the favicon should be.

In addition to the fix, there needs to be some kind of test to alert us if this breaks again.

 
Screen Shot 2017-12-06 at 12.18.05 PM.png
29.3 KB View Download
Summary: [macViewsBrowser] Tab favicons are missing (was: [macVIewsBrowser] Tab favicons are missing)
Blocking: 671916
Labels: Pri-2
Status: Available (was: Untriaged)
Thanks for all of these! I'll do a big triage of  Issue 671916  blockers when the current phase is complete. See go/macviewstracking
Labels: M-68
[Bulk Edit]
Applying M-68 milestone per email discussion with ellyjones@. Pls change it if milestone is incorrectly applied. 
Labels: -Pri-2 -M-68 Target-67 Pri-1
Owner: ellyjo...@chromium.org
Status: Assigned (was: Available)
MacViews triage: targeting this for M67 since it's highly visible.

Comment 5 by weili@chromium.org, Mar 23 2018

Owner: weili@chromium.org
Status: Started (was: Assigned)
I can take this one if you don't mind.

Comment 6 by gov...@chromium.org, Mar 26 2018

Cc: ellyjo...@chromium.org
Labels: M-67
 weili@, is there nay progress here?

Note: M67 branch is coming soon on April 12th and this is BLOCKING canary/dev experiment as it is P1. 
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 29 2018

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

commit e67dc3f09004bb560d682c6ed4026a73aec4af83
Author: Wei Li <weili@chromium.org>
Date: Thu Mar 29 03:21:53 2018

Allow MacViews to display favicons

Use the same function to retrieve favicon for web contents for MacViews
as on the other platforms, so the favicon can be displayed.

BUG= 792605 

Change-Id: Ide93ddb52747f86e04d09f7741af6e881d5374d6
Reviewed-on: https://chromium-review.googlesource.com/979258
Commit-Queue: Wei Li <weili@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546724}
[modify] https://crrev.com/e67dc3f09004bb560d682c6ed4026a73aec4af83/chrome/browser/ui/tab_ui_helper.cc
[modify] https://crrev.com/e67dc3f09004bb560d682c6ed4026a73aec4af83/chrome/browser/ui/views/frame/browser_view_browsertest.cc

Comment 9 by gov...@chromium.org, Mar 29 2018

Is anything pending or this can be marked as fixed?
Status: Fixed (was: Started)
This looks fixed in trunk.
Cc: krajshree@chromium.org
Labels: Needs-Feedback
Unable to reproduce the issue on mac 10.12.6 using chrome reported version #65.0.3287.0as per comment #0. Observed that there is no blank space at the favicon place after the spinner finishes loading.

Same behaviour in observed in latest chrome version #67.0.3390.0 after enabling flag MacViews-Browser i.e here is no blank space at the favicon place after the spinner finishes loading.
Attached a screen cast for reference.

weili@ - Could you please check the attached screen cast and please help us in confirming the fix.

Thanks...!!



792605.mp4
714 KB View Download
Labels: -Needs-Feedback
krajshree@, the video showed Chrome version 67.0.3390.0. You need to try version before 67.0.3384.0 to reproduce the problem and make sure --enable-features=ViewsBrowserWindows is there.
Labels: TE-Verified-M67 TE-Verified-67.0.3396.0
Able to reproduce the issue using chrome version #67.0.3382.0(without fix) by enabling flag --enable-features=ViewsBrowserWindows. 

Verified the fix on Mac 10.12.6 using Chrome version #67.0.3396.0 as per the comment #0 and #12.
Attaching screen cast for reference.
Observed that after loading spinner in the tab finished spinning a favicon appeared.
Hence, the fix is working as expected. 
Adding the verified labels.

Thanks...!!
792605@M67.mp4
486 KB View Download

Sign in to add a comment