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

Issue 903806 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 6
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug

Blocking:
issue 903825
issue 911327



Sign in to add a comment

[Tab loading animation] Update favicon display animation to spec

Project Member Reported by edwardjung@chromium.org, Nov 9

Issue description

Currently the favicon cross fades in with the placeholder.

We would like to proceed with animating the favicon in from the top per the proposal.

Original proposal:
https://docs.google.com/presentation/d/1r_63uPXs--gqO-FcA8fmzEuPavWrU3YJBkSObpo-D60/edit#slide=id.g2eec932b24_0_30

Spec:
https://docs.google.com/document/d/1zLDEQquWVCLmA28Ir-RBavjVhAyfs-4JEman65aNuO4/edit?usp=sharing




 
Blocking: 903825
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 28

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

commit 6c86c85c5f86a8261f7bd380146a57c1c5758099
Author: Peter Boström <pbos@chromium.org>
Date: Wed Nov 28 00:54:52 2018

Animate in favicon from the top

Adds a drop-in motion to the current fade-in effect. This (fade in and
motion) is currently linear but expected to be updated in a later change
after consulting UX.

Bug:  chromium:903806 
Change-Id: Iebc44c281ad18be2c02a1e84903cc6d0ff64b84f
Reviewed-on: https://chromium-review.googlesource.com/c/1352689
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611475}
[modify] https://crrev.com/6c86c85c5f86a8261f7bd380146a57c1c5758099/chrome/browser/ui/views/tabs/tab_icon.cc
[modify] https://crrev.com/6c86c85c5f86a8261f7bd380146a57c1c5758099/chrome/browser/ui/views/tabs/tab_icon.h

Status: Started (was: Assigned)
Great job, refinement from Helene per spec:
- Opacity of the placeholder to fade out to 0 over 200ms rather than just disappearing.


Hi, two things I noticed:

1.) The drop-in animation sometimes doesn't show up - instead the favicon appears immediately. Couldn't identify when it is the case :(

2.) The drop-in animation stutters on my MacBookAir and isn't smooth every time, which feels a little bit distractive then in the corner of my eyes :(

Thanks for listening :)
@mehmet, could you get a screen recording for us for both these cases?

It should animate every time, even on refresh.

Thanks.
Hi edwardjung: Please find the screencast attached.

In the first tab you'll see that the drop-in stutters (like there is a short break in the animation). In the second tab, you'll see that the favicon appears without the drop-in animation (sorry the cursor was shortly on it).

If you need more information, please let me know.

Thanks,
Mehmet
screenrecording.mov
396 KB View Download
Okay thanks. That is a little weird. 

I see you are visiting a site where we will have cached the icon but it should still animate in. Does it happen to all sites which you've been to before or just specific ones like Google.de?

What model of MBA do you have? I'll do a test on my aging one at home. 
I am using a MacBookAir 11" Mid 2012 (Non-Retina). Yes, I see this mostly, when I am visiting a site on the second tab where the same icon is still animating in on the first tab. I also could repro with macrumors.com or youtube.com.

Just let me know, if you need more info.
Okay I just checked this on my 2010 MBA 11" and I'm not experiencing this issue, the animation is being shown and smooth. 

Could you file a separate bug. 

Maybe try in incognito and a clean profile just to rule out extension interference. 
I'd assume the break in the animation is because there's no frame update, they should be happening at 30fps, but could probably be starved by the loading page, for instance.
re#4 - re#10: I opened  issue 910847  and  issue 910848 . Thanks.
Re #10, it seems like this is a actual bug, per the steps outlined in  issue 910848 , I was able to reproduce on my MBP. It's possible the slow loading page is taking up all the resources causing this jank. 

Jank due to resource contention w/ other things running on of the machine is unfortunately way out of scope here. :(

In the screen recording above, 2. not showing an animation is probably / possibly issue 900036.
Project Member

Comment 14 by bugdroid1@chromium.org, Dec 4

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

commit 76e02a5b59bd965376946da324fcabd671c12ee2
Author: Peter Boström <pbos@chromium.org>
Date: Tue Dec 04 19:46:57 2018

Add fade-in/fade-out to the favicon placeholder

Bug:  chromium:903806 ,  chromium:907494 
Change-Id: Iad7e8f22630b842071e16f606a9fb2632415d967
Reviewed-on: https://chromium-review.googlesource.com/c/1359300
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613655}
[modify] https://crrev.com/76e02a5b59bd965376946da324fcabd671c12ee2/chrome/browser/ui/views/tabs/tab_icon.cc
[modify] https://crrev.com/76e02a5b59bd965376946da324fcabd671c12ee2/chrome/browser/ui/views/tabs/tab_icon.h

Labels: Merge-Request-72
Requesting merge for #14, verified in Canary.
Project Member

Comment 16 by sheriffbot@chromium.org, Dec 6

Labels: -Merge-Request-72 Hotlist-Merge-Approved Merge-Approved-72
Your change meets the bar and is auto-approved for M72. Please go ahead and merge the CL to branch 3626 manually. Please contact milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 17 by bugdroid1@chromium.org, Dec 6

Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b9b7993b62b6fc90ce6610f65ee59d53d5deedc2

commit b9b7993b62b6fc90ce6610f65ee59d53d5deedc2
Author: Peter Boström <pbos@chromium.org>
Date: Thu Dec 06 17:54:45 2018

Add fade-in/fade-out to the favicon placeholder

Bug:  chromium:903806 ,  chromium:907494 
Change-Id: Iad7e8f22630b842071e16f606a9fb2632415d967
Reviewed-on: https://chromium-review.googlesource.com/c/1359300
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#613655}(cherry picked from commit 76e02a5b59bd965376946da324fcabd671c12ee2)
Reviewed-on: https://chromium-review.googlesource.com/c/1365978
Reviewed-by: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#116}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/b9b7993b62b6fc90ce6610f65ee59d53d5deedc2/chrome/browser/ui/views/tabs/tab_icon.cc
[modify] https://crrev.com/b9b7993b62b6fc90ce6610f65ee59d53d5deedc2/chrome/browser/ui/views/tabs/tab_icon.h

Status: Fixed (was: Started)
I'll mark this as fixed as it's covering too much including M72 branches. Non-linear favicon drop in is now  issue 912620 .
Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/b9b7993b62b6fc90ce6610f65ee59d53d5deedc2

Commit: b9b7993b62b6fc90ce6610f65ee59d53d5deedc2
Author: pbos@chromium.org
Commiter: pbos@chromium.org
Date: 2018-12-06 17:54:45 +0000 UTC

Add fade-in/fade-out to the favicon placeholder

Bug:  chromium:903806 ,  chromium:907494 
Change-Id: Iad7e8f22630b842071e16f606a9fb2632415d967
Reviewed-on: https://chromium-review.googlesource.com/c/1359300
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#613655}(cherry picked from commit 76e02a5b59bd965376946da324fcabd671c12ee2)
Reviewed-on: https://chromium-review.googlesource.com/c/1365978
Reviewed-by: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#116}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

Sign in to add a comment