Jared Wein [:jaws] (please needinfo? me) Reporter
	Description • 10 years ago

Just noticed today in the March 14, 2014 Nightly build. See attached image.

The favicon looks to be squished vertically and repeated three times horizontally. The bottom half of the favicon has some negative/inverted looking graphic.

	Jared Wein [:jaws] (please needinfo? me) Reporter
	Comment 1 • 10 years ago

It may be HiDPI only.

	Brian R. Bondy [:bbondy] Assignee
	Comment 2 • 10 years ago

I can reproduce this on my laptop in both Aurora and Nightly but not Beta. 
I *think* only in high DPI as well.

	Brian R. Bondy [:bbondy] Assignee
	Comment 3 • 10 years ago

Hi Tim,

So the story here is that a long time ago we used EncodeAndScaleImage functionality when encoding of icons was on the main thread, but eventually encoding data was moved off the main thread.  There's no functionality as of right now for encoding a buffer of data which you must pass resized (You can't pass an image container that was created on a diff thread).

So the best thing here is to encode the icon at the same size and let windows resize.

	Brian R. Bondy [:bbondy] Assignee
	Comment 4 • 10 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/cecf52b88021

	Brian R. Bondy [:bbondy] Assignee
	Comment 5 • 10 years ago

Is there a side effect that you're particularly concerned about jwatt? I was going to actually request that this be uplifted to aurora and beta.

	Brian R. Bondy [:bbondy] Assignee
	Comment 6 • 10 years ago

(I think it happens on beta as well, I just didn't reproduce it there)

	Jonathan Watt [:jwatt]
	Comment 7 • 10 years ago

I'm not sure what you mean. I've not expressed any concern about this patch in this bug. I've only requested we track for Aurora, which would seem like an indication of support. :)

In bug 993302 I do wonder whether we should perhaps keep the scaling behavior, but I think it's fine to just do that on Trunk, if at all.

	Brian R. Bondy [:bbondy] Assignee
	Comment 8 • 10 years ago

Yep sorry, I meant to ask if you had any concern with the me tracking for beta as well, that perhaps you knew about within the blocking bug, but it sounded wrong :)

	Jonathan Watt [:jwatt]
	Comment 9 • 10 years ago

The patch will be different for beta, but no, I've no concerns about that.

	Brian R. Bondy [:bbondy] Assignee
	Comment 10 • 10 years ago

I just noticed that I was thinking a different bug caused the problem than you.
You mentioned in the other bug that it was bug 981430.
I was thinking bug 870529

	Jonathan Watt [:jwatt]
	Comment 11 • 10 years ago

The fix for bug 870529 landed landed almost a year ago, whereas the fix for bug 981430 landed just less than a month ago.

Jared, do you have any insight into whether this is an old or recent problem? (I'm rebuilding my Windows VM ATM so can't test.)

	Carsten Book [:Tomcat]
	Comment 12 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/cecf52b88021

	Jared Wein [:jaws] (please needinfo? me) Reporter
	Comment 13 • 10 years ago

(In reply to Jonathan Watt [:jwatt] from comment #11)
> The fix for bug 870529 landed landed almost a year ago, whereas the fix for
> bug 981430 landed just less than a month ago.
> 
> Jared, do you have any insight into whether this is an old or recent
> problem? (I'm rebuilding my Windows VM ATM so can't test.)

I tried to run a mozregression search for this but I don't see any jumplists in any of the builds that it gave me between 2012-01-01 and 2014-02-09. Maybe we only show jumplists for installed builds instead of unpackaged builds?

	Brian R. Bondy [:bbondy] Assignee
	Comment 14 • 10 years ago

I think only installed builds yes.  It has to do with a property set on the icon that sets its app user model id.

	Jonathan Watt [:jwatt]
	Comment 15 • 10 years ago

Comment on attachment 8402154 [details] [diff] [review]
Fix - rev1.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 870529 or bug 981430
User impact if declined: broken jumplist icons
Testing completed (on m-c, etc.): been on m-c a day or two
Risk to taking this patch (and alternatives if risky): should be low
String or IDL/UUID changes made by this patch: none

	Jonathan Watt [:jwatt]
	Comment 16 • 10 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/101252a08e36

	Cornel Ionce [:noni] [Hubs QA]
	Comment 17 • 10 years ago

Mozilla/5.0 (Windows NT 6.3; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0

Verified on Firefox 31 beta 7, build ID: 20140703154127 using a Microsoft Surface Pro 2 device.
There are no graphic glitches displayed in the favicons anymore.