1401122 - The favicon of a pinned tab displays glitches when hovered or brought to focus

Status: NEW

(Core :: Layout, defect, P3)

Description

[Andrei Vaida [:avaida]]

Attachment: screencast

[Affected versions]:
- DevEdition 57.0b1-build3 (20170917031738)
- Nightly 57.0a1 (20170918220054)

[Affected platforms]:
- Ubuntu 16.04 x64
- Windows 10 x64
- Windows 7 x64
- Mac OS X 10.11

[Steps to reproduce]:
1. Launch Firefox.
2. Open a few tabs, e.g. Reddit, Facebook, Gmail, Instagram.
3. Pin all these tabs.
4. Open another tab but do not pin it and keep it in focus.
5. Hover each of the pinned tabs repeatedly, especially Reddit and Gmail.

[Expected result]:
- Each of the pinned tabs are hovered as expected in terms of styling and this action does not affect the favicon.

[Actual result]:
- Hovering or bringing a pinned tab into focus has a strange effect on the website's favicon, it looks like the favicon has its height slightly changed and the quality of the favicon itself is displayed as inferior -- all in all, it looks like a glitch.

[Regression range]:
- This is the best I could do with mozregression.
- Last good: 52285ea5e54c73d3ed824544cef2ee3f195f05e6 (2017-08-02).
- First bad: 63e261ce8cb04c913d2e6b19ea451b7078d24dc1 (2017-08-03).
- Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=52285ea5e54c73d3ed824544cef2ee3f195f05e6&tochange=63e261ce8cb04c913d2e6b19ea451b7078d24dc1

[Additional notes]:
- This is not reproducible on Firefox 56 and older versions.
- The favicons from Gmail and Reddit are the ones that show this glitch most easily.
- If you can't reproduce by hover, try to bring each of the pinned tabs to focus.

Comment 1

[:Gijs (he/him)]

Dupe of bug 1401037, maybe? Can you retest with a nightly that has the fix for that bug?

Comment 2

[Markus Stange [:mstange]]

This smells like bug 852754, which should be fixed...

As for what caused this, it was bug 1349555 which added the tab hover animation.

Comment 3

[Andrei Vaida [:avaida]]

(In reply to :Gijs from comment #1)
> Dupe of bug 1401037, maybe? Can you retest with a nightly that has the fix
> for that bug?

Checked this again on Windows 10 using Nightly 57.0a1 (2017-09-21) and the issue is still reproducible, here's a second screencast: https://i.imgur.com/ntF0hlE.gif.

Comment 5

[Andrei Vaida [:avaida]]

Attachment: screencast 58.0a1 (2017-09-25)

A quick update here, this bug is reproducible on Nightly 58.0a1 (2017-09-25) - see attachment, and Beta 57.0b3-build1 (20170925150345).

On Nightly 58 it's easier to reproduce, happens on hover using (e.g.) Facebook.

On Beta 57 is significantly harder to reproduce, I was actually unable to catch it in a screencast. It's only reproducible there when bringing the tab into focus and it happens after several tries.

Comment 6

[Dão Gottwald [:dao]]

Like bug 1387296 this feels like a Gecko bug.

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

[Tracking Requested - why for this release]: Requesting tracking for a visual glitch that would be introduced in 57 but based on a layout bug that has existed for potentially longer than 57.

Comment 8

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Tracked for 57 since it's a new regression. Hi Jet, to save time and increase the likelihood of getting this fixed in 57, would you be able to find an owner for this soon? Thanks.

Comment 9

[Jet Villegas (inactive)]

Andre: can you try toggling image.downscale-during-decode.enabled to false? That seems to "fix" this for me, but that could mean that Layout is computing different icon dimensions on hover, which would be wrong in this case.

Comment 10

[Jet Villegas (inactive)]

Andrew: can you track down where the favicon size is computed to help diagnosis? Thx!

Comment 11

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Attachment: Screencast on HiDPI display with no obvious glitching

I've been unable to reproduce this on Windows 10 with Nightly, Beta, or DevEdition, maximized and not maximized. Jet suggested I post a screencast of it NOT glitching for comparison.

Perhaps this only reproduces on low dpi displays?

Comment 12

[Alice0775 White]

I can reproduce the glitch on 58.0a1 Windows10.
It is regardless of Theme and window size mode.
It happens when layout.css.devPixelsPerPx less than 1.02 for me.

Reproducible: always

Steps To Reproduce:
1. Open and login and pinned the following pages. (Read the tab if highlighted the pinned tab)
   https://mail.google.com/
   https://www.facebook.com/
   New Tab
2. Open any page in normal tab and select this tab
3. Restart Browser and restore previous session
4. Mouse move very slowly from the normal tab to the left
 

Actual Results:
Favicon of pinned tab temporarily shrink a pixel in vertically or horizontally.

Comment 13

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=0fdb6a37c65caaffbcf04834589c13bda0b96cd4&tochange=9aea49fa17a10967b4c28d7519ab1ea1ed27b9b6

Regressed by: bug 1349555

Comment 14

[Jet Villegas (inactive)]

It appears to only affect favicons from .ico files though it's not clear why the same icon in unpinned tabs are unaffected. There are a number of CSS changes to pinned tabs in bug 1349555 and it would be good to isolate the specific change that regressed this.

For example, this selector was removed from browser/base/content/tabbrowser.css:

.tab-stack {
  vertical-align: top; /* for pinned tabs */
}

Comment 15

[Dão Gottwald [:dao]]

(In reply to Jet Villegas (:jet) from comment #14)
> It appears to only affect favicons from .ico files though it's not clear why
> the same icon in unpinned tabs are unaffected. There are a number of CSS
> changes to pinned tabs in bug 1349555 and it would be good to isolate the
> specific change that regressed this.
> 
> For example, this selector was removed from
> browser/base/content/tabbrowser.css:
> 
> .tab-stack {
>   vertical-align: top; /* for pinned tabs */
> }

Like bug 1387296, this is caused by the hover transition for the line at the top of tabs:

https://searchfox.org/mozilla-central/rev/20e41d4a61a8f5e34c9cf357304b78b3e9bced8a/browser/themes/shared/tabs.inc.css#489

Comment 16

[Andrew Osmond [:aosmond] (he/him)]

(In reply to Jet Villegas (:jet) from comment #10)
> Andrew: can you track down where the favicon size is computed to help
> diagnosis? Thx!

I'm not sure if I am the best person to ask. Imagelib will say the size is just the largest resource size in the ICO, but the favicon service does some processing/repacking of them, and presumably aids in the selection of the "best" size via nsFaviconService::PreferredSizeFromURI.

I think we were looking at ComputeSnappedImageDrawingParameters in bug 1398697 by causing off-by-one pixel differences for SVGs and what could be done about that. Perhaps this is related?

Comment 17

[Jet Villegas (inactive)]

(In reply to Andrew Osmond [:aosmond] from comment #16)
> I think we were looking at ComputeSnappedImageDrawingParameters in bug
> 1398697 by causing off-by-one pixel differences for SVGs and what could be
> done about that. Perhaps this is related?

It does seem like it from the factors that "fix" it:
1. disable downscaling ( comment 9 )
2. use a high-res display ( comment 11 )
3. remove a CSS transition ( comment 15 )

I think the transition is just a trigger that causes Layout to compute intermediate sizes that snap differently depending on the number of device pixels available. #1 & #2 also "fix" the issue in bug 1404366.

Comment 18

[Marco Bonardo [:mak]]

(In reply to Andrew Osmond [:aosmond] from comment #16)
> I'm not sure if I am the best person to ask. Imagelib will say the size is
> just the largest resource size in the ICO, but the favicon service does some
> processing/repacking of them, and presumably aids in the selection of the
> "best" size via nsFaviconService::PreferredSizeFromURI.

That's not a problem for the tab icon, we don't do any preprocessing or selection for the tab icon, indeed the tab loads the icon by itself setting directly it as the image attribute.
There's no intervention at all of the favicons service on the tab icon.

Comment 19

[Markus Stange [:mstange]]

This is a problem with layerization. The icon renders differently when rendered as an ImageLayer vs when it's drawn into a PaintedLayer. The patch in bug 852754 did not fix this because it only avoids ImageLayers if the image has been scaled down to 0.3 times its size or smaller. The reddit icon is 32x32 and only gets scaled down to 0.5 times its size, so we use an ImageLayer for it if the icon does not share its layer with anything else.

I don't know what to suggest. The patch in bug 1387296 "fixes" this issue as well. I don't see a way to fix this on the Gecko side in any short term.

Comment 20

[Markus Stange [:mstange]]

Attachment: display list excerpt + layers without the workaround from 1387296

Comment 21

[Markus Stange [:mstange]]

Attachment: display list excerpt + layers with the workaround from 1387296

Comment 22

[Marco Bonardo [:mak]]

May we have a Try build with the workaround, so we can ask people to test if it solves their problem?
There are various bugs around where favicons disappears or is not shown at all if there are pinned tabs and it may be interesting to understand if there's a relation.

Comment 23

[Markus Stange [:mstange]]

(In reply to Marco Bonardo [::mak] from comment #22)
> May we have a Try build with the workaround, so we can ask people to test if
> it solves their problem?

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ade5a8bf5f8ca53a4f59086334a7c8d5f7a09dae

Comment 24

[Marco Bonardo [:mak]]

Ok, this build and it doesn't fix bug 1404366. I posted the link to bug 1405253 and bug 1403963 to see if there's any relation.
Though, I also suspect a network code problem (bug 1406091).

Comment 25

[Liz Henry (:lizzard) (relman/hg->git project)]

Sounds like this may be related to bug 1391797 as well.

Comment 26

[Jet Villegas (inactive)]

Attachment: Screen Recording 2017-20-12

(In reply to Marco Bonardo [::mak] from comment #24)
> Ok, this build and it doesn't fix bug 1404366. I posted the link to bug
> 1405253 and bug 1403963 to see if there's any relation.
> Though, I also suspect a network code problem (bug 1406091).

The posted build does fix this bug 1401122 as seen in the attachment "Screen Recording 2017-20-12." The left window is today's Nightly and the right window is the posted Try build. 

Note the mask in the icon from bug 1404366 is bad in both builds, though that's not related to this bug.

Comment 27

[Liz Henry (:lizzard) (relman/hg->git project)]

Too late for a fix for 57 but we could still potentially take a patch in 58. I don't think relman needs to track this issue.

Comment 28

[Bogdan Maris, Desktop QA [Holiday+PTO 1-6 May]]

I was able to reproduce this bug using the latest Nightly only on Win 10 x64 and Mac OS X 10.11, macOS 10.12 but not on Ubuntu 12.04 x32, Ubuntu 16.04 x64.  When I set the image.downscale-during-decode.enabled to false, I couldn't reproduce this issue (valid for every platform). 

Note: It seems that the glitches are more visible when hovering from right to left.