1137615 - Some icons are not displayed in the bookmarks sidebar

Status: RESOLVED FIXED

Description

[Andrew I.R.]

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:35.0) Gecko/20100101 Firefox/35.0
Build ID: 20150122214805

Steps to reproduce:

1. Open the bookmarks sidebar (Ctrl-B)
2. Create a new folder in the Bookmarks Menu.
3. Copy (just copy / paste) into this folder following URLs (it's only sample, problem occured with some other URLs too):
---
http://forum.guns.ru/forumtopics/143.html
http://forum.guns.ru/forumtopics/342.html
http://forum.guns.ru/forumtopics/94.html
http://forum.guns.ru/forumtopics/343.html
http://forum.guns.ru/forumtopics/64.html
http://forum.guns.ru/forumtopics/5.html
http://forum.guns.ru/forumtopics/308.html
http://forum.guns.ru/forumtopics/182.html
http://forum.guns.ru/forumtopics/200.html
http://forum.guns.ru/forummessage/84/98822.htm
http://forum.guns.ru/forumindex/7.html
---

4. Open all these bookmarks to get favicons.
5. Close opened tabs, close the folder (!), restart Firefox.
6. Open the folder.


Actual results:

A blank space instead of icons. The icons appear when you move the mouse over the bookmarks.


Expected results:

Icons instead of blank space.

Comment 1

[Andrew I.R.]

Also affected 37 and 38 versions as far as I understand.

Comment 2

[a.v.procenko]

Also icons disappears in ScrapBook addons and bookmarks.

PS Developer Scrapbook X inform, that because new API applyed in FF36??

Comment 3

[Andrew I.R.]

It seems that the problem occurs with sites having image/x-icon favicon.
Other sites having this issue,for example.
http://www.vmware.com/
http://slashdot.jp/
http://gigazine.net/
http://news.mynavi.jp/
http://www.iiyama.co.jp/
http://jpn.nec.com/

Comment 4

[Andrew I.R.]

Confirmation of this version is the fact that a direct replacement icons from ICO to PNG (places.sqlite->moz_favicons->data and mime_type) solves the problem. And vice versa - replacement from PNG to ICO reproduces the effect of invisibility.

Comment 5

[a.v.procenko]

And what about Scrapbook icons?

PS how i can change ICO>PNG through this addon? https://addons.mozilla.org/uk/firefox/addon/sqlite-manager/

Comment 6

[Andrew I.R.]

I do not use SB, sorry.

For changing FF favicons, you must export places.sqlite->moz_favicons->data as file, convert it, using any graphics editor and import back to same place. Also you must change places.sqlite->moz_favicons->mime_type from image/x-icon to image/png.

Comment 7

[a.v.procenko]

Вы говорите по русски?)

Сделал как Вы говорили для теста с пропаданием, у меня все иконки на месте. Но проподяют в Скепбуке каждый раз как захожу в него, нужно провести мышкой по ним или открыть\закрыть папки.

Вы простите. Нельзя ли по подробнее чем открыть places.sqlite и как поменять image/x-icon в image/png.

Comment 8

[Andrew I.R.]

Вы мне е-мэйлом бы писали тогда, а то нерусскоязычные точно будут эту запись игнорировать :)

Я говорю, что не знаком со Скрэпбуком лично, следовательно ничего про него сказать не могу, увы.

А вот SQLITE manager-ом и открыть. Там залезаете в таблицу moz_favicons, ищете свою иконку, сохраняете её (поле data) в файл, конвертируете этот файл чем-нибудь в PNG (у меня, скажем, GIMP), и импортируете обратно. А в поле mime_type просто текст - его и надо с "image/x-icon" на "image/png" поменять. Операция, увы, поштучная.. я, во всяком случае, не нашёл никаких пакетных файловых операций в SM.

Comment 9

[a.v.procenko]

Ясно, спасибо). Буду пробовать. Жаль, что целиком нельзя все поменять(

Comment 10

[Luís Miguel [:quicksaver] (not reading bugmail)]

This doesn't seem like a problem with the bookmarks system itself, but rather some problem with the rendering.

Comment 11

[a.v.procenko]

(In reply to Luís Miguel [:Quicksaver] from comment #10)
> This doesn't seem like a problem with the bookmarks system itself, but
> rather some problem with the rendering.

And what we need to do?

ps https://youtu.be/bygYl6-2jHI

Comment 12

[atlanto]

Platforms in addition.
Windows 8.1 x64
Windows 7 Home Premium x64
Linux Mint 17.1 Cinnamon 64-bit
Linux Mint 17.1 Cinnamon 32-bit

There is a bit different behaviour on Linux.
The problematic icon is not displayed when I opened(expanded) the bookmark folder, but it is displayed just when I move mouse pointer outside the bookmarks tree area.
(other triggers - mousehover, close/reopen the bookmark folder, expand another bookmark folder, start Firefox with expanded bookmark folder - are same as Windows)

Comment 13

[void73]

I would also like to confirm this bug on the following OS (on 4 different hardware configs):
Windows 7 Professional x64
Windows 7 Home Premium x64
Linux Mint 17.1 Cinnamon 32-bit

For Linux Mint, a new, unmodified install was used to confirm this bug.

The bug is present in 36 and 36.0.1. I never saw it in older versions and did not check with newer builds.

The defective favicons seem to be limited to "image/x-icon".

This bug may not break FF, but it is quite ugly to have icons pop in all the time in the sidebar after every FF start.

Comment 14

[atlanto]

firefox-36.0.1.source.tar.bz2 and firefox-36.0.4.source.tar.bz2
in 'image' dir
decoders/nsICODecoder.cpp:88:    mInvalidRect.Union(mContainedDecoder->TakeInvalidRect());
decoders/nsICODecoder.cpp:602:  mInvalidRect.Union(mContainedDecoder->TakeInvalidRect());
decoders/nsICODecoder.cpp:650:    mInvalidRect.Union(mContainedDecoder->TakeInvalidRect());
src/RasterImage.cpp:2659:      invalidRect.Union(decoder->TakeInvalidRect());
src/RasterImage.cpp:2685:    mNotifyInvalidRect.Union(invalidRect);

should be
decoders/nsICODecoder.cpp:88:    mInvalidRect = mInvalidRect.Union(mContainedDecoder->TakeInvalidRect());
decoders/nsICODecoder.cpp:602:  mInvalidRect = mInvalidRect.Union(mContainedDecoder->TakeInvalidRect());
decoders/nsICODecoder.cpp:650:    mInvalidRect = mInvalidRect.Union(mContainedDecoder->TakeInvalidRect());
src/RasterImage.cpp:2659:      invalidRect = invalidRect.Union(decoder->TakeInvalidRect());
src/RasterImage.cpp:2685:    mNotifyInvalidRect = mNotifyInvalidRect.Union(invalidRect);

(compiled and confirmed/verified on LinuxMint)

Comment 15

[atlanto]

Attachment: patch

Comment 16

[Seth Fowler [:seth] [:s2h]]

Comment on attachment 8582935 [details] [diff] [review]
patch

Review of attachment 8582935 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for tracking this down.

We need to ensure that we get a warning when the return value of a method like Union() is unused. We should have caught this automatically.

Comment 17

[Seth Fowler [:seth] [:s2h]]

(In reply to Seth Fowler [:seth] from comment #16)
> We need to ensure that we get a warning when the return value of a method
> like Union() is unused. We should have caught this automatically.

To follow up: in bug 1147706 I'm adding annotations that will make us warn when the return value of Union() and similar methods is ignored. I've confirmed that they would have caught this bug.

Comment 18

[Seth Fowler [:seth] [:s2h]]

Pushed to inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/8ab8a64af8ec

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8ab8a64af8ec

Comment 20

[a.v.procenko]

And how fix in 36.0.4?

Comment 21

[Timothy Nikkel (:tnikkel)]

Looks like bug 1100725 added the problematic code.

Comment 22

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8582935 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 1100725
[User impact if declined]: images whose filetype is icon may not draw properly, like in the bookmarks bar
[Describe test coverage new/current, TreeHerder]: nothing specifically
[Risks and why]: low risk
[String/UUID change made/needed]: none

Comment 23

[Timothy Nikkel (:tnikkel)]

(In reply to a.v.procenko from comment #20)
> And how fix in 36.0.4?

Generally for small fixes like this we don't make a new release on the release channel. (ie no 36.0.5) But I've nominated the fix for beta and aurora. It's most likely too late for 37 as that gets released on tuesday. If you want to use Firefox with the fix you can either use Firefox Nightly now, wait until the first Firefox beta 38 comes out in a week or so, use Fireox Aurora (developer edition) when the fix gets uplifted there (likely less than a week), or wait until 38 comes out on May 11.

Comment 24

[a.v.procenko]

I try 39 nightly, and i have same icon disappear in Scarpbook that i have in 36.0.4(((

Comment 25

[Seth Fowler [:seth] [:s2h]]

(In reply to a.v.procenko from comment #24)
> I try 39 nightly, and i have same icon disappear in Scarpbook that i have in
> 36.0.4(((

Yes, the patch didn't make the last Nightly build. It only landed this morning. It should be in tonight's Nightly.

Comment 26

[Sylvestre Ledru [:sylvestre]]

Comment on attachment 8582935 [details] [diff] [review]
patch

Too late for 37.

Comment 27

[Ryan VanderMeulen [:RyanVM]]

(In reply to Seth Fowler [:seth] from comment #18)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/8ab8a64af8ec

In the future, please be more careful about double-checking commit information for submitted patches.

Comment 28

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d7ec6454a5c9

Comment 29

[Seth Fowler [:seth] [:s2h]]

Talked with Ryan about the issue with the commit information in that patch, which I didn't immediately notice.

Andrew, you may want to set a valid email address for usage with hg.

Comment 30

[a.v.procenko]

in aurora FF38 Scrapbook icons working good.
but what with FF36? waiting until FF38?

Comment 31

[Seth Fowler [:seth] [:s2h]]

(In reply to a.v.procenko from comment #30)
> in aurora FF38 Scrapbook icons working good.
> but what with FF36? waiting until FF38?

Unfortunately the fix won't arrive until FF38, yeah. (Which means 6 weeks until it lands in a release.) The change just missed FF37.

Comment 32

[Andrew I.R.]

I confirm that it's fixed in version 38 beta.
The question is closed, thanks to all who took part in the discussion (and all developers, of course :).

Comment 33

[Andrew I.R.]

Version 43 has the same or a similar error(42 it was not). If plaсes.sglite-> moz_faviсons-> mime_tipe is image/PNG, and the date is gif file, icon only appears when holding the mouse cursor over it. If the file in the database manyally replaced by true PNG - the effect disappears.

Comment 34

[Andrew I.R.]

Confirmed in 43.0 - 43.0.3

Comment 35

[Andrew I.R.]

All favicons must be converted in the PNG format, all it's neverendless story.

Comment 36

[Andrew I.R.]

"all it's" == "otherwise it's", sorry